search

DerekStride / tree-sitter-sql

SQL grammar for tree-sitter
http://derek.stride.host/tree-sitter-sql/
MIT License
147 stars 47 forks source link

fix: Change the parser to require a semicolon in function_body of 1 s… #216

Closed antoineB closed 9 months ago

antoineB commented 10 months ago

…tatement.

In order to reduce the number of parser state the grammar are less complient, now the function_body of 1 statement needs to end with a semicolon.

Idealy the parse of the function_body should be done by another parser in regard of the language defined.

antoineB commented 10 months ago

see https://github.com/DerekStride/tree-sitter-sql/issues/209

DerekStride commented 10 months ago

If semi-colons are optional in this location I don't know if we should enforce them to simplify the parser. That said our parser is pretty large so I see why it's appealing. Are there any other thoughts?

Perhaps it's time to split the function body parsing into a separate parser?

dmfay commented 10 months ago

yeah, the semicolon is really optional here and if we require it we will violate at least Postgres' expectations and I think the standard. It's worth looking imo at the state (as it were) of this parser relative to others. Here's a not very scientific sampling:

parser state count large state count
Java 1381 406
JavaScript 2781 440
Fortran 2921 447
Rust 3227 782
LaTeX 3371 2022
Common Lisp 4165 405
SQLite (dhcmrlchtdj) 5043 6
Ruby 5815 2122
Bash 6164 426
Elixir 6986 1091
COBOL (Sakamoto) 7744 5095
C++ 8043 2123
SQL (Stride) 14149 187
Scala 18538 2415
SQL (Novikov) 19852 4737
Haskell 22407 2777
Kotlin 25983 15612
Julia 31161 15000

We're above the median and mean, but it's hard to draw an exact parallel between SQL and other kinds of languages (COBOL might come the closest), and we aim to support several SQL dialects. The main effect of high state counts is annoying development experience. We should try to keep it down but I don't think this is a crisis. Also worth noting our very low large state count compared to almost everything else.

Julia people are used to waiting forever for things to precompile ;)

matthias-Q commented 9 months ago

I would suggest we close this PR. I think keeping it optional is the right choice.