Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FORMATTING] comments after final WITH statement #660

Open
BFJonk opened this issue Nov 9, 2023 · 3 comments
Open

[FORMATTING] comments after final WITH statement #660

BFJonk opened this issue Nov 9, 2023 · 3 comments
Labels

Comments

@BFJonk
Copy link

BFJonk commented Nov 9, 2023

Input data

Which SQL and options did you provide as input?

/* fix late arriving client dimension keys */
     with result as (
          -- do the update
             update star.fact_user fu
                set dim_client_key = dim_client_hist.dim_client_key
               from star.dim_client_hist
              where
                    -- match business keys
                    fu.client_id = dim_client_hist.id
                    -- pick correct version
                and fu.date between dim_client_hist.dss_start_date and dim_client_hist.dss_end_date
                    -- only update these records
                and fu.dim_client_key = 0
                and fu.client_id is not null
          returning 1
          )
        , updated_rows as (
          -- count the updated records
             select count(*) as count
               from result
          )
          -- return success message   
   select 1 as p_status
        , (
          'Updated ' || updated_rows.count || ' records from star.fact_user with late arriving dimensional keys from star.dim_client_hist'
          )::varchar as p_return_msg
     from updated_rows
;

Expected Output
I expected the comment in line 22 to be aligned with the select afte the comment

/* fix late arriving client dimension keys */
     with result as (
          -- do the update
             update star.fact_user fu
                set dim_client_key = dim_client_hist.dim_client_key
               from star.dim_client_hist
              where
                    -- match business keys
                    fu.client_id = dim_client_hist.id
                    -- pick correct version
                and fu.date between dim_client_hist.dss_start_date and dim_client_hist.dss_end_date
                    -- only update these records
                and fu.dim_client_key = 0
                and fu.client_id is not null
          returning 1
          )
        , updated_rows as (
          -- count the updated records
             select count(*) as count
               from result
          )
-- return success message   
   select 1 as p_status
        , (
          'Updated ' || updated_rows.count || ' records from star.fact_user with late arriving dimensional keys from star.dim_client_hist'
          )::varchar as p_return_msg
     from updated_rows
;

NB. Not sure if this will start to cause other problems if you manage to fix this
PS. the as statements (aliases) in line 23 & line 26 are also not aligned

Actual Output

/* fix late arriving client dimension keys */
     with result as (
          -- do the update
             update star.fact_user fu
                set dim_client_key = dim_client_hist.dim_client_key
               from star.dim_client_hist
              where
                    -- match business keys
                    fu.client_id = dim_client_hist.id
                    -- pick correct version
                and fu.date between dim_client_hist.dss_start_date and dim_client_hist.dss_end_date
                    -- only update these records
                and fu.dim_client_key = 0
                and fu.client_id is not null
          returning 1
          )
        , updated_rows as (
          -- count the updated records
             select count(*) as count
               from result
          )
          -- return success message   
   select 1 as p_status
        , (
          'Updated ' || updated_rows.count || ' records from star.fact_user with late arriving dimensional keys from star.dim_client_hist'
          )::varchar as p_return_msg
     from updated_rows

Usage

  • How are you calling / using the library?

image

  • What SQL language(s) does this apply to?
    At least PostgreSQL

  • Which SQL Formatter version are you using?
    Online version
    image

@BFJonk BFJonk added the bug label Nov 9, 2023
@nene
Copy link
Collaborator

nene commented Nov 9, 2023

Thanks for reporting. Ignoring all the irrelevant SQL code, this issue comes down to:

select col
-- comment
from tbl;

getting formatted as:

select
  col
  -- comment
from
  tbl;

instead of:

select
  col
-- comment
from
  tbl;

This is indeed unfortunate. It happens because the formatter really only looks at the tokens that preceded the comment, it's not really aware of the code that's following. Implementing this sort of lookahead is possible... but it's a major undertaking. Unlikely to happen. Instead I'm concentrating on implementing an entirely new SQL formatting tool, which should solve most of the shortcomings of the SQL Formatter.

Regarding the indentation of AS-keywords, this feature is known to be buggy and will likely be removed entirely in the future.

@BFJonk
Copy link
Author

BFJonk commented Nov 20, 2023

Hi @nene / Rene,

Thanks for answering. Nice you're working on a new version. Unfortunate that a lookahead is difficult. Few questions:

  • The first link has no options yet
  • It also has no PostgreSQL variant yet

I guess these are still to come :-)

  • The second link leads to a 404
  • What do you mean by, 'the indentation of AS-keywords will be removed'? Will the AS keyword lead to a syntax error?

@nene
Copy link
Collaborator

nene commented Nov 20, 2023

The first link has no options yet

There are a few options. Like the option to uppercase keywords. (Not accessible from the demo page though). But the goal with this tool is to have a bare minimum of formatting options. Following more in the footsteps of the Prettier formatting tool.

It also has no PostgreSQL variant yet

Yep, that's still in the works, because I first need to implement a parser for PostgreSQL.

The second link leads to a 404
What do you mean by, 'the indentation of AS-keywords will be removed'? Will the AS keyword lead to a syntax error?

Sorry about that. I already dropped this feature in master so the link was broken as a result. Fixed it to point at 13.x version.

The AS-keyword can be used just fine. Only the feature that indents AS keywords ("Align aliases" checkbox in your screenshot) has been removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants