What is wrong with this syntax? Query will work, but CTE will not "compile"

  • with UnloadDates asselect DISTINCT ShipmentID,

    (select Min(starttime)

    from tblInvoice I

    where I.ShipmentID = O.shipmentID

    and DataSent is null

    ) StartTime,

    (select Max(Endtime)

    from tblInvoice I

    where I.ShipmentID = O.shipmentID

    and DataSent is null

    ) EndTime

    from tblInvoice O

    where O.startTime IS NOT NULL

    and O.EndTime IS NOT NULL

    and O.DataSent is null

    Msg 156, Level 15, State 1, Line 4

    Incorrect syntax near the keyword 'select'.

    Appreciate the help

    <><
    Livin' down on the cube farm. Left, left, then a right.

  • Hrm. Seems like your CTE declaration is missing a few pieces; I believe it should be like so:

    with UnloadDates(ShipmentId,StartTime,EndTime) as(

    select DISTINCT ShipmentID,

    (select Min(starttime)

    from tblInvoice I

    where I.ShipmentID = O.shipmentID

    and DataSent is null

    ) StartTime,

    (select Max(Endtime)

    from tblInvoice I

    where I.ShipmentID = O.shipmentID

    and DataSent is null

    ) EndTime

    from tblInvoice O

    where O.startTime IS NOT NULL

    and O.EndTime IS NOT NULL

    and O.DataSent is null)

    The CTE needs to have a column listing in the format of: CTEName(Column1,Column2,...). After the AS, you need a starting parenthesis, which you close at the end of your CTE query.

    Post back if it's still giving you issues; I believe I caught the errors, but there might be something I overlooked.

    EDIT: Doh, forgot the closing parenthesis.

    - 😀

  • hisakimatama (4/8/2014)


    EDIT: Doh, forgot the closing parenthesis.

    The parenthesis where the problem, a CTE does not require the columnar declaration.

    😎

  • Since you're correcting this code, why don't we make it simpler and faster?

    WITH UnloadDates AS(

    SELECT ShipmentID,

    MIN(starttime) StartTime,

    MAX(Endtime) EndTime

    FROM tblInvoice O

    WHERE O.startTime IS NOT NULL

    AND O.EndTime IS NOT NULL

    AND O.DataSent is null

    GROUP BY ShipmentID

    )

    SELECT ShipmentID,

    StartTime,

    EndTime

    FROM UnloadDates

    This way you read the table just once instead of trice. It's also a lot less code. 🙂

    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
  • Arrgh. Sorry. I keep thinking those are input parameters, not output.

    Another issue I had is that you can not just "compile" (execute) a CTE definition by itself without some follow on expressions. :Whistling:

    <><
    Livin' down on the cube farm. Left, left, then a right.

  • Luis Cazares (4/8/2014)


    Since you're correcting this code, why don't we make it simpler and faster?

    This way you read the table just once instead of trice. It's also a lot less code. 🙂

    Oye. I didn't write it that way because I have a memory, evidently faulty, that that would not work. But it does. And since it does, I might not even need the CTE.

    You guys (which includes gals) are the best!!!!

    <><
    Livin' down on the cube farm. Left, left, then a right.

  • Tobar (4/8/2014)


    Arrgh. Sorry. I keep thinking those are input parameters, not output.

    Another issue I had is that you can not just "compile" (execute) a CTE definition by itself without some follow on expressions. :Whistling:

    Joke?

    If not, that's because a CTE is just a query, a derived table, no different than if you did a SELECT FROM a SELECT FROM. The CTE only exists within the context of it's following statement.

    If it was a joke, sorry for getting pedantic.

    "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

  • Alas ignorance, possibly just not thinking it through, but most likely ignorance.:blush:

    <><
    Livin' down on the cube farm. Left, left, then a right.

  • Eirikur Eiriksson (4/8/2014)


    hisakimatama (4/8/2014)


    EDIT: Doh, forgot the closing parenthesis.

    The parenthesis where the problem, a CTE does not require the columnar declaration.

    😎

    Aha, so it doesn't :-). I've always declared CTEs with the column declarations, and by the wonderful problems of habit, it stuck as "the" way to do it. Well, I learned something that should make syntax a good bit clearer myself 😀

    - 😀

  • hisakimatama (4/8/2014)


    Aha, so it doesn't :-). I've always declared CTEs with the column declarations, and by the wonderful problems of habit, it stuck as "the" way to do it. Well, I learned something that should make syntax a good bit clearer myself 😀

    I prefer it, makes the code more readable.

    😎

  • Eirikur Eiriksson (4/8/2014)


    hisakimatama (4/8/2014)


    Aha, so it doesn't :-). I've always declared CTEs with the column declarations, and by the wonderful problems of habit, it stuck as "the" way to do it. Well, I learned something that should make syntax a good bit clearer myself 😀

    I prefer it, makes the code more readable.

    😎

    That is just an opinion. There are times when it makes sense and times when it doesn't.

    However, every column in a cte MUST be named. If you have a derived column of some sort if MUST have a name.

    In the following you will see I have a column with the constant 'asdf' but the column has no name. This will not parse.

    with MyCte as

    (

    select top 5 'asdf'

    , name

    from sys.objects

    )

    select * from MyCte;

    But, simply add a column alias and it is fine.

    with MyCte as

    (

    select top 5 'asdf' as MyColumn

    , name

    from sys.objects

    )

    select * from MyCte;

    _______________________________________________________________

    Need help? Help us help you.

    Read the article at http://www.sqlservercentral.com/articles/Best+Practices/61537/ for best practices on asking questions.

    Need to split a string? Try Jeff Modens splitter http://www.sqlservercentral.com/articles/Tally+Table/72993/.

    Cross Tabs and Pivots, Part 1 – Converting Rows to Columns - http://www.sqlservercentral.com/articles/T-SQL/63681/
    Cross Tabs and Pivots, Part 2 - Dynamic Cross Tabs - http://www.sqlservercentral.com/articles/Crosstab/65048/
    Understanding and Using APPLY (Part 1) - http://www.sqlservercentral.com/articles/APPLY/69953/
    Understanding and Using APPLY (Part 2) - http://www.sqlservercentral.com/articles/APPLY/69954/

  • Sean Lange (4/8/2014)


    Eirikur Eiriksson (4/8/2014)


    hisakimatama (4/8/2014)


    Aha, so it doesn't :-). I've always declared CTEs with the column declarations, and by the wonderful problems of habit, it stuck as "the" way to do it. Well, I learned something that should make syntax a good bit clearer myself 😀

    I prefer it, makes the code more readable.

    😎

    That is just an opinion. There are times when it makes sense and times when it doesn't.

    However, every column in a cte MUST be named. If you have a derived column of some sort if MUST have a name.

    In the following you will see I have a column with the constant 'asdf' but the column has no name. This will not parse.

    with MyCte as

    (

    select top 5 'asdf'

    , name

    from sys.objects

    )

    select * from MyCte;

    But, simply add a column alias and it is fine.

    with MyCte as

    (

    select top 5 'asdf' as MyColumn

    , name

    from sys.objects

    )

    select * from MyCte;

    Thanks Sean, I should have been more clear on that.

    😎

  • Eirikur Eiriksson (4/8/2014)


    hisakimatama (4/8/2014)


    Aha, so it doesn't :-). I've always declared CTEs with the column declarations, and by the wonderful problems of habit, it stuck as "the" way to do it. Well, I learned something that should make syntax a good bit clearer myself 😀

    I prefer it, makes the code more readable.

    😎

    I've found just the opposite to be true if there are a ton of columns being returned by the CTE. I'd just as soon write it as a good and proper encapsulated SELECT and let the defintion of the CTE be simple.

    But, to each their own! I even change my ways when I'm working for a different company and they say how they want it done. 😛

    --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)

  • Jeff Moden (4/27/2014)


    Eirikur Eiriksson (4/8/2014)


    hisakimatama (4/8/2014)


    Aha, so it doesn't :-). I've always declared CTEs with the column declarations, and by the wonderful problems of habit, it stuck as "the" way to do it. Well, I learned something that should make syntax a good bit clearer myself 😀

    I prefer it, makes the code more readable.

    😎

    I've found just the opposite to be true if there are a ton of columns being returned by the CTE. I'd just as soon write it as a good and proper encapsulated SELECT and let the defintion of the CTE be simple.

    But, to each their own! I even change my ways when I'm working for a different company and they say how they want it done. 😛

    Let me rephrase it, I prefer it, when it makes the code more readable:-P

  • Eirikur Eiriksson (4/27/2014)


    Jeff Moden (4/27/2014)


    Eirikur Eiriksson (4/8/2014)


    hisakimatama (4/8/2014)


    Aha, so it doesn't :-). I've always declared CTEs with the column declarations, and by the wonderful problems of habit, it stuck as "the" way to do it. Well, I learned something that should make syntax a good bit clearer myself 😀

    I prefer it, makes the code more readable.

    😎

    I've found just the opposite to be true if there are a ton of columns being returned by the CTE. I'd just as soon write it as a good and proper encapsulated SELECT and let the defintion of the CTE be simple.

    But, to each their own! I even change my ways when I'm working for a different company and they say how they want it done. 😛

    Let me rephrase it, I prefer it, when it makes the code more readable:-P

    I usually make sure to name all the columns in the select list but there are times when I define the list of columns separately but not sure when or why. I have actually gotten away from it since working with Oracle for a year as the subquery refactoring clause doesn't support the definition of the column names upfront.

Viewing 15 posts - 1 through 15 (of 16 total)

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