Inconsistency in identifier tokens

[jessestimpson]

Hi,

This library has unlocked the ability for me to add a SQL layer to FoundationDB (something often asked for by FDB community), via EctoFoundationDB. I'm in the very early and experimental stages of converting the parsed SQL from this library into an Ecto.Query that the EctoFoundationDB Adapter can convert into the FoundationDB transaction. For now I'm choosing not to pull :sql as a dep into EctoFDB; using Ecto.Query as the standard representation for the time being.

I am wondering if you consider it safe for me to inspect the parsed representation of the SQL output by this library, and whether or not that representation is reasonably stable that I can develop against it? I'm using undocumented APIs to do it currently. 😅

I've noticed one inconsistency so far, perhaps it's a bug? Notice that the 'id' identifier is represented as {:ident, _, ["id"]} and 'name' as {:name, _, []}.

iex(1)> {:ok, _opts, _, _, _, _, tokens} = SQL.Lexer.lex("select id from my_table", {1, 0, nil}, 0, format: true)
iex(2)> SQL.Parser.parse(tokens)
[
  {:select, [line: 0, column: 6, file: {1, 0, nil}],
   [{:ident, [line: 0, column: 9, file: {1, 0, nil}], ["id"]}]},
  {:from, [line: 0, column: 14, file: {1, 0, nil}],
   [{:ident, [line: 0, column: 23, file: {1, 0, nil}], ["my_table"]}]}
]
iex(3)> {:ok, _opts, _, _, _, _, tokens} = SQL.Lexer.lex("select name from my_table", {1, 0, nil}, 0, format: true)
iex(4)> SQL.Parser.parse(tokens)
[
  {:select, [line: 0, column: 6, file: {1, 0, nil}],
   [{:name, [line: 0, column: 11, file: {1, 0, nil}], []}]},
  {:from, [line: 0, column: 16, file: {1, 0, nil}],
   [{:ident, [line: 0, column: 25, file: {1, 0, nil}], ["my_table"]}]}
]

Thanks for creating this project!

[Schultzer]

Thanks for giving this a spin!

Before I released the first version I had experimented with translating Ecto.Query to and from SQL and having a library that could translate both directions would be very useful!

The reason why you’re seeing {:name, _, []} is because name is a keyword, although I need to check the grammar for how it is used, since in this case it should probably be treated as an ident.

I’m sure you can see the idea with the AST and it is undocumented because there might be some of these inconsistencies, although it does not affect the parser as much as it would for someone relying on the AST.

At some point I would like to be able to leverage more of the rules to handle these cases.

I’m not planning to make too many changes to the AST, there might be nodes that would be nested in the future and more metadata included.

[Schultzer]

It’s gonna be a bit before I get around to this one as I’m going on vacation, the immediate fix on your end would be to encapsulate your ident, for Postgres syntax it’s double quotes.

I did take a look at the grammar file and it looks like I have to deal with the rest of the rules sooner rather than later. Which is both daunting and great since we will have improved compile time errors 😎