Skip to main content
Code Review

Return to Answer

replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link
  1. In the first half of the UNION, you join with APA_T, which is...

     SELECT DISTINCT apa.district AS district,
     (SELECT s1.activity_package_id
     FROM tbl_activity_package_address s1
     WHERE apa.district = s1.district
     ORDER BY s1.id DESC
     LIMIT 1
     ) AS idActivityPackage
     FROM tbl_activity_package_address apa 
     ORDER BY apa.district
    

    The DISTINCT there is superfluous, since the innermost SELECT is guaranteed to return at most one row per district.

    As I read it, APA_T is a subquery that lists the most recently added activity_package_id for each district. I think it's worth creating a view for clarity.

     CREATE VIEW latest_activity_package_address AS
     SELECT *
     FROM tbl_activity_package_address
     WHERE id IN (
     SELECT MAX(id)
     FROM tbl_activity_package_address AS latest_apa
     GROUP BY district
     );
    

    Then, the first half of the UNION becomes:

     SELECT GROUP_CONCAT(district ORDER BY district), t.name
     FROM tbl_activity AS t
     INNER JOIN tbl_activity_package AS ap
     ON t.id = ap.activity_id
     INNER JOIN latest_activity_package_address AS apa
     ON ap.id = apa.activity_package_id
     WHERE (ap.publication_date <= CURDATE())
     AND (DATE_ADD(ap.publication_date, INTERVAL ap.publication_duration_in_days DAY) >= CURDATE())
     GROUP BY t.name
     ORDER BY apa.district
    

    I've copied the ORDER BY clause into the GROUP_CONCAT(), where I think it belongs.

  2. Next, I'd like to point out that LIMIT 6,6 will produce non-deterministic results, since there is no ORDER BY clause on the UNION query. You've put an ORDER BY on each half of the UNION, but that doesn't guarantee that the result will be ordered the same way that doesn't guarantee that the result will be ordered the same way.

  3. Surprisingly, I haven't found a way to reduce duplication between the two halves of the UNION. However, there is now a nice parallelism which may be more aesthetically pleasing. You don't need to select new table aliases for the second half; they aren't in the same scope as the first half.

     SELECT GROUP_CONCAT(district ORDER BY district), t.name
     FROM tbl_activity AS t
     INNER JOIN tbl_activity_package AS ap
     ON t.id = ap.activity_id
     INNER JOIN latest_activity_package_address AS apa
     ON ap.id = apa.activity_package_id
     WHERE (ap.publication_date <= CURDATE())
     AND (DATE_ADD(ap.publication_date, INTERVAL ap.publication_duration_in_days DAY) >= CURDATE())
     GROUP BY t.name
     ORDER BY apa.district
     UNION DISTINCT
     SELECT GROUP_CONCAT(district ORDER BY district), t.name
     FROM tbl_activity AS t
     INNER JOIN tbl_activity_package AS ap
     ON t.id = ap.activity_id
     INNER JOIN tbl_activity_package_address AS apa
     ON ap.id = apa.activity_package_id
     WHERE (ap.publication_date <= CURDATE())
     AND (DATE_ADD(ap.publication_date, INTERVAL ap.publication_duration_in_days DAY) >= CURDATE())
     GROUP BY t.name
     ORDER BY apa.district
    
  1. In the first half of the UNION, you join with APA_T, which is...

     SELECT DISTINCT apa.district AS district,
     (SELECT s1.activity_package_id
     FROM tbl_activity_package_address s1
     WHERE apa.district = s1.district
     ORDER BY s1.id DESC
     LIMIT 1
     ) AS idActivityPackage
     FROM tbl_activity_package_address apa 
     ORDER BY apa.district
    

    The DISTINCT there is superfluous, since the innermost SELECT is guaranteed to return at most one row per district.

    As I read it, APA_T is a subquery that lists the most recently added activity_package_id for each district. I think it's worth creating a view for clarity.

     CREATE VIEW latest_activity_package_address AS
     SELECT *
     FROM tbl_activity_package_address
     WHERE id IN (
     SELECT MAX(id)
     FROM tbl_activity_package_address AS latest_apa
     GROUP BY district
     );
    

    Then, the first half of the UNION becomes:

     SELECT GROUP_CONCAT(district ORDER BY district), t.name
     FROM tbl_activity AS t
     INNER JOIN tbl_activity_package AS ap
     ON t.id = ap.activity_id
     INNER JOIN latest_activity_package_address AS apa
     ON ap.id = apa.activity_package_id
     WHERE (ap.publication_date <= CURDATE())
     AND (DATE_ADD(ap.publication_date, INTERVAL ap.publication_duration_in_days DAY) >= CURDATE())
     GROUP BY t.name
     ORDER BY apa.district
    

    I've copied the ORDER BY clause into the GROUP_CONCAT(), where I think it belongs.

  2. Next, I'd like to point out that LIMIT 6,6 will produce non-deterministic results, since there is no ORDER BY clause on the UNION query. You've put an ORDER BY on each half of the UNION, but that doesn't guarantee that the result will be ordered the same way.

  3. Surprisingly, I haven't found a way to reduce duplication between the two halves of the UNION. However, there is now a nice parallelism which may be more aesthetically pleasing. You don't need to select new table aliases for the second half; they aren't in the same scope as the first half.

     SELECT GROUP_CONCAT(district ORDER BY district), t.name
     FROM tbl_activity AS t
     INNER JOIN tbl_activity_package AS ap
     ON t.id = ap.activity_id
     INNER JOIN latest_activity_package_address AS apa
     ON ap.id = apa.activity_package_id
     WHERE (ap.publication_date <= CURDATE())
     AND (DATE_ADD(ap.publication_date, INTERVAL ap.publication_duration_in_days DAY) >= CURDATE())
     GROUP BY t.name
     ORDER BY apa.district
     UNION DISTINCT
     SELECT GROUP_CONCAT(district ORDER BY district), t.name
     FROM tbl_activity AS t
     INNER JOIN tbl_activity_package AS ap
     ON t.id = ap.activity_id
     INNER JOIN tbl_activity_package_address AS apa
     ON ap.id = apa.activity_package_id
     WHERE (ap.publication_date <= CURDATE())
     AND (DATE_ADD(ap.publication_date, INTERVAL ap.publication_duration_in_days DAY) >= CURDATE())
     GROUP BY t.name
     ORDER BY apa.district
    
  1. In the first half of the UNION, you join with APA_T, which is...

     SELECT DISTINCT apa.district AS district,
     (SELECT s1.activity_package_id
     FROM tbl_activity_package_address s1
     WHERE apa.district = s1.district
     ORDER BY s1.id DESC
     LIMIT 1
     ) AS idActivityPackage
     FROM tbl_activity_package_address apa 
     ORDER BY apa.district
    

    The DISTINCT there is superfluous, since the innermost SELECT is guaranteed to return at most one row per district.

    As I read it, APA_T is a subquery that lists the most recently added activity_package_id for each district. I think it's worth creating a view for clarity.

     CREATE VIEW latest_activity_package_address AS
     SELECT *
     FROM tbl_activity_package_address
     WHERE id IN (
     SELECT MAX(id)
     FROM tbl_activity_package_address AS latest_apa
     GROUP BY district
     );
    

    Then, the first half of the UNION becomes:

     SELECT GROUP_CONCAT(district ORDER BY district), t.name
     FROM tbl_activity AS t
     INNER JOIN tbl_activity_package AS ap
     ON t.id = ap.activity_id
     INNER JOIN latest_activity_package_address AS apa
     ON ap.id = apa.activity_package_id
     WHERE (ap.publication_date <= CURDATE())
     AND (DATE_ADD(ap.publication_date, INTERVAL ap.publication_duration_in_days DAY) >= CURDATE())
     GROUP BY t.name
     ORDER BY apa.district
    

    I've copied the ORDER BY clause into the GROUP_CONCAT(), where I think it belongs.

  2. Next, I'd like to point out that LIMIT 6,6 will produce non-deterministic results, since there is no ORDER BY clause on the UNION query. You've put an ORDER BY on each half of the UNION, but that doesn't guarantee that the result will be ordered the same way.

  3. Surprisingly, I haven't found a way to reduce duplication between the two halves of the UNION. However, there is now a nice parallelism which may be more aesthetically pleasing. You don't need to select new table aliases for the second half; they aren't in the same scope as the first half.

     SELECT GROUP_CONCAT(district ORDER BY district), t.name
     FROM tbl_activity AS t
     INNER JOIN tbl_activity_package AS ap
     ON t.id = ap.activity_id
     INNER JOIN latest_activity_package_address AS apa
     ON ap.id = apa.activity_package_id
     WHERE (ap.publication_date <= CURDATE())
     AND (DATE_ADD(ap.publication_date, INTERVAL ap.publication_duration_in_days DAY) >= CURDATE())
     GROUP BY t.name
     ORDER BY apa.district
     UNION DISTINCT
     SELECT GROUP_CONCAT(district ORDER BY district), t.name
     FROM tbl_activity AS t
     INNER JOIN tbl_activity_package AS ap
     ON t.id = ap.activity_id
     INNER JOIN tbl_activity_package_address AS apa
     ON ap.id = apa.activity_package_id
     WHERE (ap.publication_date <= CURDATE())
     AND (DATE_ADD(ap.publication_date, INTERVAL ap.publication_duration_in_days DAY) >= CURDATE())
     GROUP BY t.name
     ORDER BY apa.district
    
Source Link
200_success
  • 145.5k
  • 22
  • 190
  • 478

Your SQL Fiddle and the question have inconsistent column names: the fiddle has id_activity_package, while the code in this question has activity_package_id. I'll consider the fiddle as supplementary information, and treat the code in the question as authoritative.


The two halves of the UNION are nearly identical, with the exception of the second INNER JOIN. Let's analyze it one step at a time.

  1. In the first half of the UNION, you join with APA_T, which is...

     SELECT DISTINCT apa.district AS district,
     (SELECT s1.activity_package_id
     FROM tbl_activity_package_address s1
     WHERE apa.district = s1.district
     ORDER BY s1.id DESC
     LIMIT 1
     ) AS idActivityPackage
     FROM tbl_activity_package_address apa 
     ORDER BY apa.district
    

    The DISTINCT there is superfluous, since the innermost SELECT is guaranteed to return at most one row per district.

    As I read it, APA_T is a subquery that lists the most recently added activity_package_id for each district. I think it's worth creating a view for clarity.

     CREATE VIEW latest_activity_package_address AS
     SELECT *
     FROM tbl_activity_package_address
     WHERE id IN (
     SELECT MAX(id)
     FROM tbl_activity_package_address AS latest_apa
     GROUP BY district
     );
    

    Then, the first half of the UNION becomes:

     SELECT GROUP_CONCAT(district ORDER BY district), t.name
     FROM tbl_activity AS t
     INNER JOIN tbl_activity_package AS ap
     ON t.id = ap.activity_id
     INNER JOIN latest_activity_package_address AS apa
     ON ap.id = apa.activity_package_id
     WHERE (ap.publication_date <= CURDATE())
     AND (DATE_ADD(ap.publication_date, INTERVAL ap.publication_duration_in_days DAY) >= CURDATE())
     GROUP BY t.name
     ORDER BY apa.district
    

    I've copied the ORDER BY clause into the GROUP_CONCAT(), where I think it belongs.

  2. Next, I'd like to point out that LIMIT 6,6 will produce non-deterministic results, since there is no ORDER BY clause on the UNION query. You've put an ORDER BY on each half of the UNION, but that doesn't guarantee that the result will be ordered the same way.

  3. Surprisingly, I haven't found a way to reduce duplication between the two halves of the UNION. However, there is now a nice parallelism which may be more aesthetically pleasing. You don't need to select new table aliases for the second half; they aren't in the same scope as the first half.

     SELECT GROUP_CONCAT(district ORDER BY district), t.name
     FROM tbl_activity AS t
     INNER JOIN tbl_activity_package AS ap
     ON t.id = ap.activity_id
     INNER JOIN latest_activity_package_address AS apa
     ON ap.id = apa.activity_package_id
     WHERE (ap.publication_date <= CURDATE())
     AND (DATE_ADD(ap.publication_date, INTERVAL ap.publication_duration_in_days DAY) >= CURDATE())
     GROUP BY t.name
     ORDER BY apa.district
     UNION DISTINCT
     SELECT GROUP_CONCAT(district ORDER BY district), t.name
     FROM tbl_activity AS t
     INNER JOIN tbl_activity_package AS ap
     ON t.id = ap.activity_id
     INNER JOIN tbl_activity_package_address AS apa
     ON ap.id = apa.activity_package_id
     WHERE (ap.publication_date <= CURDATE())
     AND (DATE_ADD(ap.publication_date, INTERVAL ap.publication_duration_in_days DAY) >= CURDATE())
     GROUP BY t.name
     ORDER BY apa.district
    
lang-sql

AltStyle によって変換されたページ (->オリジナル) /