Bug: Select of functions get reordered, breaking comments and whole query

[quassy]

What Happened

playground

SELECT
    name AS name1,
    name2,  -- my fav function
    FAVORITE('%y%m%j', exit_date) AS quite_long_column_name,
    -- my newest function    NEWEST('%y%m%j', exit_date)
        AS quite_long_column_name,
    PARSE_DATE('%y%m%j', exit_date) AS quite_long_column_name
FROM users

locally with v0.19.0

SELECT
    name AS name1,
    name2,  -- my fav function
    FAVORITE('%y%m%j', exit_date) AS quite_long_column_name,
    -- my newest function
    NEWEST('%y%m%j', exit_date) AS quite_long_column_name,
    PARSE_DATE('%y%m%j', exit_date) AS quite_long_column_name,
FROM USERS

Expected Behaviour

1. Column order should not be changed (just because some columns use a function).
2. Inline comments should keep their context.
3. Long lines with a comment should not break the resulting SQL (only reproducible in playground).

How to reproduce

SELECT 
    name AS name1,
    FAVORITE('%y%m%j', exit_date) AS quite_long_column_name,  -- my fav function
    NEWEST('%y%m%j', exit_date) AS quite_long_column_name,  -- my newest function
    PARSE_DATE('%y%m%j', exit_date) AS quite_long_column_name,
    name2
from USERS

Configuration

[sqruff]
dialect = bigquery
rules = all

[benfdking]

Hi @quassy,

Thanks for filing the bug report. A few thoughts.

Reordering

I expect your config to reorder the columns because of the rules = all config line. That activates the structure.column_order rule which explicitly does this.

That said, by default, I think the playground should only include the core rules, so we'll change that.

Comments causing issues

That's a bug that we will need to investigate. It's quite a tricky one that one but we'll dig.

Comment context

Keeping comment and context together in a reorder is tricky because we yet don't have a definition of what ties a comment to a particular context.

[benfdking]

#995

[quassy]

I expect your config to reorder the columns because of the rules = all config line. That activates the structure.column_order rule which explicitly does this.

Thanks! Oversight by me.

Keeping comment and context together in a reorder is tricky because we yet don't have a definition of what ties a comment to a particular context.

Not sure how you can parse it but at least for inline comments like SELECT t.firstname || t.name AS n -- get full name the comment should try to stay with the last token(?) of the line, so in this case the alias n.

But even this could be tricky:

SELECT 
    t.firstname  -- legal first name
    || t.name AS n

For comments on their own line like this

SELECT
    t.lastname as l,
    -- get the name
    t.firstname as f,

I guess there could be two configuration options: keep comments with line above (so try to stay as close as possible to l while being its own line), keep comments with line below (so try to stay as close as possible to t.firstname while being on its own line).

But even there I don't how to best deal with this case:

SELECT
    t.lastname
    -- get the last name
    as l,
    t.firstname as f,