Skip to content

blank lines not properly parsed in 0.3.1 #345

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

Closed
mmoncure opened this issue Apr 15, 2025 · 2 comments
Closed

blank lines not properly parsed in 0.3.1 #345

mmoncure opened this issue Apr 15, 2025 · 2 comments
Labels
bug Something isn't working

Comments

@mmoncure
Copy link

parsing the following valid SQL:

  SELECT

    a,
    b,

    c,
    d
  FROM foo f;

...yields

DT7FN214T3:processor merlin.moncure$ ~/pgtools/pgtools check --skip-db ~/pgtools/test.sql
/Users/merlin.moncure/pgtools/test.sql:3:5 syntax ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

  ✖ Invalid statement: syntax error at or near "a"

    1 │   SELECT
    2 │
  > 3 │     a,
      │     ^^
  > 4 │     b,
      │     ^^
    5 │
    6 │     c,


/Users/merlin.moncure/pgtools/test.sql:6:5 syntax ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

  ✖ Invalid statement: syntax error at or near "c"

    4 │     b,
    5 │
  > 6 │     c,
      │     ^^
  > 7 │     d
  > 8 │   FROM foo f;·
      │   ^^^^^^^^^^^
    9 │


Checked 1 file in 1399µs. No fixes applied.
Found 2 errors.
check ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

  ✖ Some errors were emitted while running checks.

@mmoncure mmoncure added the bug Something isn't working label Apr 15, 2025
@psteinroe
Copy link
Collaborator

hey! thanks for reporting. this is actually by design. we spent a long time trying to figure out how to split a potentially invalid source file into individual statements. we ended up with a naive approach that tries to be "smart" about common statements but always splits at double newlines (and semicolons ofc) as a fallback. you can find more about the rationale in our blog post.

closing this in favour of #135

@merlinm
Copy link

merlinm commented Apr 16, 2025

Ok thanks. Yeah this makes sense, FWIW, this issue (along with a few others that were raised and closed) were the only ones that were raised out of checking a pretty large codebase. So generally speaking I think you've done really well.

I do think ultimately you'd be better off exposing the client side lexer from libpq which would then manage all of these considerations (we briefly discussed this in another topic) which would give provable parity with server processing at a statement level which INSHO ought to be a design goal; I suspect there may be other edge cases lurking. Either way, I'm a huge fan of this project and will continue to support as I'm able.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants