Why does this amended JOIN cause a syntax error.

  • I wish to change this SQL:

    SELECT
        CONVERT(varchar(2), Data.WeekNumber) AS WeekNumber,
        CONVERT(DECIMAL(18,2),SUM(Purchases)) AS Purchases,
        CONVERT(DECIMAL(18,2), SUM(Data.PurchasesMinusWarrantyParts)) AS PurchasesMinusWarrantyParts,
        CONVERT(DECIMAL(18, 2), SUM(Targets.Objective)) AS Objective,
        CONVERT(DECIMAL(18, 1),(FLOOR(CASE WHEN SUM(ISNULL(Objective, 0)) = 0 THEN NULL ELSE SUM(PurchasesMinusWarrantyParts) / SUM(Objective) END * 10000) / 100)) AS Performance

    FROM
        [dbo].[Dates] AS Dates LEFT OUTER JOIN
        [dbo].[Kpis_PPGVPData] AS Data ON Data.WeekNumber = Dates.WeekNumber AND Data.YearId = Dates.Year LEFT OUTER JOIN
        [dbo].[Kpis_Targets] AS Targets ON Data.YearId = Targets.YearId AND Data.SiteId = Targets.SiteId AND isnull(Data.SaleTypeId,0) = isnull(Targets.SaleTypeId,0) AND Data.WeekNumber = Targets.WeekNumber LEFT OUTER JOIN
        [dbo].Sites Sites ON ISNULL(Data.SiteId,Targets.SiteId) = Sites.SiteId INNER JOIN [dbo].SiteOrgLevel1 SiteOrgLevel1 ON Sites.SiteOrgLevel1Id = SiteOrgLevel1.SiteOrgLevel1Id
    GROUP BY
      Data.WeekNumber

    to this:

    SELECT
        CONVERT(varchar(2), Data.WeekNumber) AS WeekNumber,
        CONVERT(DECIMAL(18,2),SUM(Purchases)) AS Purchases,
        CONVERT(DECIMAL(18,2), SUM(Data.PurchasesMinusWarrantyParts)) AS PurchasesMinusWarrantyParts,
        CONVERT(DECIMAL(18, 2), SUM(Targets.Objective)) AS Objective,
        CONVERT(DECIMAL(18, 1),(FLOOR(CASE WHEN SUM(ISNULL(Objective, 0)) = 0 THEN NULL ELSE SUM(PurchasesMinusWarrantyParts) / SUM(Objective) END * 10000) / 100)) AS Performance

    FROM
        [dbo].[Kpis_Targets] AS Targets LEFT OUTER JOIN
        [dbo].[Dates] AS Dates LEFT OUTER JOIN
        [dbo].[Kpis_PPGVPData] AS Data ON Data.WeekNumber = Dates.WeekNumber AND Data.YearId = Dates.Year LEFT OUTER JOIN
        [dbo].[SiteOrgLevel1] AS SiteOrgLevel1 INNER JOIN
        [dbo].[Sites] AS Sites ON SiteOrgLevel1.SiteOrgLevel1Id = Sites.SiteOrgLevel1Id ON ISNULL(Data.SiteId, Targets.SiteId) = Sites.SiteId ON Targets.YearId = Data.YearId AND Targets.SiteId = Data.SiteId AND ISNULL(Targets.SaleTypeId, 0) = ISNULL(Data.SaleTypeId, 0) AND Targets.WeekNumber = Data.WeekNumber
    GROUP BY
      Data.WeekNumber

    However, although the new code is accepted byQuery Designer, it doesn't execute, but instead throws an error:

    "The multi-part identifier "Targets.SiteId" could not be bound."

    I've attached a file with (I hope) sufficient DDL to build the tables.

    Many thanks in advance

    Edward

  • Edward,
    Your Joins are not in the correct order.
    SELECT
        CONVERT(varchar(2), Data.WeekNumber) AS WeekNumber,
        CONVERT(DECIMAL(18,2),SUM(Purchases)) AS Purchases,
        CONVERT(DECIMAL(18,2), SUM(Data.PurchasesMinusWarrantyParts)) AS PurchasesMinusWarrantyParts,
        CONVERT(DECIMAL(18, 2), SUM(Targets.Objective)) AS Objective,
        CONVERT(DECIMAL(18, 1),(FLOOR(CASE WHEN SUM(ISNULL(Objective, 0)) = 0 THEN NULL ELSE SUM(PurchasesMinusWarrantyParts) / SUM(Objective) END * 10000) / 100)) AS Performance
    FROM            [dbo].[Kpis_Targets] AS Targets
    LEFT OUTER JOIN [dbo].[Kpis_PPGVPData] AS Data    ON Targets.YearId = Data.YearId 
                                                      AND Targets.SiteId = Data.SiteId
                                                      AND ISNULL(Targets.SaleTypeId, 0) = ISNULL(Data.SaleTypeId, 0)
                                                      AND Targets.WeekNumber = Data.WeekNumber
    LEFT OUTER JOIN [dbo].[Dates]   AS Dates          ON Data.WeekNumber = Dates.WeekNumber
                                                      AND Data.YearId = Dates.Year
    INNER JOIN  [dbo].[Sites]   AS Sites              ON ISNULL(Data.SiteId, Targets.SiteId) = Sites.SiteId
    LEFT OUTER JOIN [dbo].[SiteOrgLevel1] AS SiteOrgLevel1 ON Sites.SiteOrgLevel1Id = SiteOrgLevel1.SiteOrgLevel1Id
    GROUP BY Data.WeekNumber

    Regards,
    Matt

  • It's because when you moved the Targets JOIN clause, you moved the corresponding ON clause to the end of the query, which means that that join will be evaluated last and the Targets alias will not be bound until that ON clause is evaluated.  You are getting the error, because you reference the Targets alias in a prior ON clause.

    There are multiple ways to fix this, but since you are using a combination of LEFT and INNER JOINS we don't have enough data to determine which of those possible changes will produce the results that you are looking for.  The easiest (but not only) way to get the correct logic when you are intermingling INNER and OUTER JOINS is to use CTEs for each of the INNER JOIN groups and use only OUTER JOINS in the main query.

    Even though this is the easiest, I don't think it's the best, but I would need more information to give you the best.

    Drew

    J. Drew Allen
    Business Intelligence Analyst
    Philadelphia, PA

  • When I write queries, I simply am NOT willing to code ANY query using any form of multiple joins prior to having an ON clause.   I have a number of really good reasons:

    1.)  Coding that way makes the query much harder to understand.
    2.) Troubleshooting a query like that becomes an order of magnitude more difficult.
    3.) Getting the right results via experimentation is much harder to achieve.
    4.) Making changes when they become necessary also gets a lot more complicated.

    Thus I always start with a base table, and work my way out from there, never using anything other than INNER, FULL OUTER, or LEFT OUTER joins, or using CROSS APPLY.   That may force me to use a CTE to manage what I'll call a certain amount of pre-processing, but in the long run, it's probably going to get me a better result and be a lot easier to fix when things go wrong, as well as when it's time to make changes.   It usually also ends up performing better because it becomes easier for the optimizer to choose a better query plan.    Mind you, there will be exceptions, but writing queries in the hardest possible way as a starting point isn't a very good coding practice, and is far more likely to lead to future problems.

  • Thank you everyone.  The problem is compounded because a) there are missing WHERE clauses that I omitted for clarity, and b) this SQL is composed within a webservice written in C#/MVC.  So there are some limitations on approach (not sure about creating a CTE within a UnitOfWork, at least the way we have designed it.  But the bones of a really good working answer are here - so thanks again!

  • sgmunson - Tuesday, May 2, 2017 12:19 PM

    When I write queries, I simply am NOT willing to code ANY query using any form of multiple joins prior to having an ON clause.   I have a number of really good reasons:

    1.)  Coding that way makes the query much harder to understand.
    2.) Troubleshooting a query like that becomes an order of magnitude more difficult.
    3.) Getting the right results via experimentation is much harder to achieve.
    4.) Making changes when they become necessary also gets a lot more complicated.

    Thus I always start with a base table, and work my way out from there, never using anything other than INNER, FULL OUTER, or LEFT OUTER joins, or using CROSS APPLY.   That may force me to use a CTE to manage what I'll call a certain amount of pre-processing, but in the long run, it's probably going to get me a better result and be a lot easier to fix when things go wrong, as well as when it's time to make changes.   It usually also ends up performing better because it becomes easier for the optimizer to choose a better query plan.    Mind you, there will be exceptions, but writing queries in the hardest possible way as a starting point isn't a very good coding practice, and is far more likely to lead to future problems.

    The multiple adjacent ON clauses are usually the consequence of using the Query Designer.  Oddly enough (but it was a very long time ago), I did find that they only appeared on certain "configurations" of joins and, when they did appear, there was actually more than a bit of performance gain.  I may have to go back to using the Query Designer for a while (hate the bloody format and always fix that, though) just to see if it shows up again so that I can have a demonstrable example on hand.

    --Jeff Moden


    RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
    First step towards the paradigm shift of writing Set Based code:
    ________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.
    "Change is inevitable... change for the better is not".

    Helpful Links:
    How to post code problems
    How to Post Performance Problems
    Create a Tally Function (fnTally)
    Intro to Tally Tables and Functions

  • Actually, I found a pretty decent explanation right here on SSC.  Chris M did a good job of it.
    https://www.sqlservercentral.com/Forums/FindPost1583604.aspx

    --Jeff Moden


    RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
    First step towards the paradigm shift of writing Set Based code:
    ________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.
    "Change is inevitable... change for the better is not".

    Helpful Links:
    How to post code problems
    How to Post Performance Problems
    Create a Tally Function (fnTally)
    Intro to Tally Tables and Functions

  • Jeff Moden - Wednesday, May 3, 2017 8:54 PM

    sgmunson - Tuesday, May 2, 2017 12:19 PM

    When I write queries, I simply am NOT willing to code ANY query using any form of multiple joins prior to having an ON clause.   I have a number of really good reasons:

    1.)  Coding that way makes the query much harder to understand.
    2.) Troubleshooting a query like that becomes an order of magnitude more difficult.
    3.) Getting the right results via experimentation is much harder to achieve.
    4.) Making changes when they become necessary also gets a lot more complicated.

    Thus I always start with a base table, and work my way out from there, never using anything other than INNER, FULL OUTER, or LEFT OUTER joins, or using CROSS APPLY.   That may force me to use a CTE to manage what I'll call a certain amount of pre-processing, but in the long run, it's probably going to get me a better result and be a lot easier to fix when things go wrong, as well as when it's time to make changes.   It usually also ends up performing better because it becomes easier for the optimizer to choose a better query plan.    Mind you, there will be exceptions, but writing queries in the hardest possible way as a starting point isn't a very good coding practice, and is far more likely to lead to future problems.

    The multiple adjacent ON clauses are usually the consequence of using the Query Designer.  Oddly enough (but it was a very long time ago), I did find that they only appeared on certain "configurations" of joins and, when they did appear, there was actually more than a bit of performance gain.  I may have to go back to using the Query Designer for a while (hate the bloody format and always fix that, though) just to see if it shows up again so that I can have a demonstrable example on hand.

    You would indeed expect there to be a performance reason to get to such nested joins.
    Such constructs resulting in good performance gain I've only seen very rare on our systems. 
    It really depends on your own system if it will result in such gains. YMMV as they say 😉

    Johan

    Learn to play, play to learn !

    Dont drive faster than your guardian angel can fly ...
    but keeping both feet on the ground wont get you anywhere :w00t:

    - How to post Performance Problems
    - How to post data/code to get the best help[/url]

    - How to prevent a sore throat after hours of presenting ppt

    press F1 for solution, press shift+F1 for urgent solution 😀

    Need a bit of Powershell? How about this

    Who am I ? Sometimes this is me but most of the time this is me

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

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