Review on SELECT query

  • In my select query, I have a part where I have written like

    SELECT A.activityCode

    ,A.ActivityName + ISNULL(A.description, '') AS ActivityName

    ,A.displayOrder

    ,A.activityStartDate

    FROM Activities AS A

    LEFT JOIN Activities AS FA

    LEFT JOIN Activities AS GFA

    LEFT JOIN Activities AS GGFA ON (

    GFA.fatheractivityIncId = GGFA.ActivityIncId

    AND GFA.fatheractivitySqlId = GGFA.activitySqlId

    AND GGFA.isDeleted = 0

    ) ON (

    FA.fatheractivityIncId = GFA.activityIncId

    AND FA.fatheractivitySqlId = GFA.activitySqlId

    AND GFA.isDeleted = 0

    ) ON (

    A.fatheractivityIncId = FA.activityIncId

    AND A.fatheractivitySqlId = FA.activitySqlId

    AND FA.isDeleted = 0

    AND A.isDeleted = 0

    )

    The person who reviewed my code, asked me

    a)To remove the braces after the ON keyword. He says that is not required. Should I remove it?

    b)3 ON keywords are there one after another. He says it is confusing and I should rewrite the query. Should I rewrite it? Is there any performance constraints?

  • Your query can be rewritten like this:

    SELECT A.activityCode

    ,A.ActivityName + ISNULL(A.description, '') AS ActivityName

    ,A.displayOrder

    ,A.activityStartDate

    FROM Activities AS A

    LEFT JOIN Activities AS FA

    ON A.fatheractivityIncId = FA.activityIncId

    AND A.fatheractivitySqlId = FA.activitySqlId

    AND FA.isDeleted = 0

    AND A.isDeleted = 0

    LEFT JOIN Activities AS GFA

    ON FA.fatheractivityIncId = GFA.activityIncId

    AND FA.fatheractivitySqlId = GFA.activitySqlId

    AND GFA.isDeleted = 0

    LEFT JOIN Activities AS GGFA

    ON GFA.fatheractivityIncId = GGFA.ActivityIncId

    AND GFA.fatheractivitySqlId = GGFA.activitySqlId

    AND GGFA.isDeleted = 0

    That way of putting ON clauses one after another is a bit confusing indeed. It is used when mixing INNER and OUTER joins, but there are clearer ways of expressing the same query.

    -- Gianluca Sartori

  • spaghettidba (10/22/2014)


    Your query can be rewritten like this:

    SELECT A.activityCode

    ,A.ActivityName + ISNULL(A.description, '') AS ActivityName

    ,A.displayOrder

    ,A.activityStartDate

    FROM Activities AS A

    LEFT JOIN Activities AS FA

    ON A.fatheractivityIncId = FA.activityIncId

    AND A.fatheractivitySqlId = FA.activitySqlId

    AND FA.isDeleted = 0

    AND A.isDeleted = 0

    LEFT JOIN Activities AS GFA

    ON FA.fatheractivityIncId = GFA.activityIncId

    AND FA.fatheractivitySqlId = GFA.activitySqlId

    AND GFA.isDeleted = 0

    LEFT JOIN Activities AS GGFA

    ON GFA.fatheractivityIncId = GGFA.ActivityIncId

    AND GFA.fatheractivitySqlId = GGFA.activitySqlId

    AND GGFA.isDeleted = 0

    That way of putting ON clauses one after another is a bit confusing indeed. It is used when mixing INNER and OUTER joins, but there are clearer ways of expressing the same query.

    Changing the order of ON clauses can change the results of your query. Unless you are exploiting this feature and know exactly what you are doing, you'd be well advised to follow spaghettidba's advice.

    Here's an example of what you can do with the ON clause:

    http://www.sqlservercentral.com/Forums/FindPost1571191.aspx

    “Write the query the simplest way. If through testing it becomes clear that the performance is inadequate, consider alternative query forms.” - Gail Shaw

    For fast, accurate and documented assistance in answering your questions, please read this article.
    Understanding and using APPLY, (I) and (II) Paul White
    Hidden RBAR: Triangular Joins / The "Numbers" or "Tally" Table: What it is and how it replaces a loop Jeff Moden

  • I also agree with Spaghetti on this one. Your approach will work. It's just not very clear. A lack of clarity can absolutely lead to error. For example, with only three tables, how hard is this to figure out? Not that bad right. But what happens when there are six tables or sixteen being joined together. Suddenly your way gets extremely confusing and therefore error prone.

    "The credit belongs to the man who is actually in the arena, whose face is marred by dust and sweat and blood"
    - Theodore Roosevelt

    Author of:
    SQL Server Execution Plans
    SQL Server Query Performance Tuning

  • Did you leave something out of your post? There seems to be no reason to have the 3 left joins in that query as it is.

    Luis C.
    General Disclaimer:
    Are you seriously taking the advice and code from someone from the internet without testing it? Do you at least understand it? Or can it easily kill your server?

    How to post data/code on a forum to get the best help: Option 1 / Option 2
  • Luis Cazares (10/22/2014)


    Did you leave something out of your post? There seems to be no reason to have the 3 left joins in that query as it is.

    Extremely good point, Luis.

    -- Gianluca Sartori

  • Luis Cazares (10/22/2014)


    Did you leave something out of your post? There seems to be no reason to have the 3 left joins in that query as it is.

    Without DISTINCT, those left joins could generate dupes in the output, which, although unlikely, could be significant.

    “Write the query the simplest way. If through testing it becomes clear that the performance is inadequate, consider alternative query forms.” - Gail Shaw

    For fast, accurate and documented assistance in answering your questions, please read this article.
    Understanding and using APPLY, (I) and (II) Paul White
    Hidden RBAR: Triangular Joins / The "Numbers" or "Tally" Table: What it is and how it replaces a loop Jeff Moden

  • ChrisM@Work (10/23/2014)


    Luis Cazares (10/22/2014)


    Did you leave something out of your post? There seems to be no reason to have the 3 left joins in that query as it is.

    Without DISTINCT, those left joins could generate dupes in the output, which, although unlikely, could be significant.

    Another good point. I bet that duplicates, if any, are hardly what the OP wants in this case.

    -- Gianluca Sartori

  • spaghettidba (10/23/2014)


    ChrisM@Work (10/23/2014)


    Luis Cazares (10/22/2014)


    Did you leave something out of your post? There seems to be no reason to have the 3 left joins in that query as it is.

    Without DISTINCT, those left joins could generate dupes in the output, which, although unlikely, could be significant.

    Another good point. I bet that duplicates, if any, are hardly what the OP wants in this case.

    Based on the column names, duplicates would indicate a bad design of the table (lack of PK) and wrong data entered as a result of that.

    Luis C.
    General Disclaimer:
    Are you seriously taking the advice and code from someone from the internet without testing it? Do you at least understand it? Or can it easily kill your server?

    How to post data/code on a forum to get the best help: Option 1 / Option 2

Viewing 9 posts - 1 through 8 (of 8 total)

You must be logged in to reply to this topic. Login to reply