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

Add conditional compilation to support different SQL dialect #207

Closed
miuy56dc opened this issue Jun 22, 2020 · 6 comments
Closed

Add conditional compilation to support different SQL dialect #207

miuy56dc opened this issue Jun 22, 2020 · 6 comments

Comments

@miuy56dc
Copy link
Contributor

miuy56dc commented Jun 22, 2020

The SQL dialect of the database is different, like when you use CREATE TABLE , if you use SQLite, maybe you don't want to use the crowd, you can write SQL like this
CREATE TABLE test ( a TEXT )WITHOUT ROWID;,
but MySQL or PostgresSQL don't support that, if you use MySQL, you can choose the storage engine like
CREATE TABLE test( a varchar(100) ) ENGINE=InnoDB;.
So, we can add features like cargo build --features "sqlite" that will support SQLite grammars.

@nickolay
Copy link
Contributor

You're right that every dialect of SQL is different. Currently (until the dialects story is figured out in #7) we deal with this by accepting dialect-specific extensions in the mainline parser -- with tests and an indication which dialects an extension applies to, but no toggle to select a dialect. Sometimes we come up with a generic grammar that the common dialects fit (e.g. ColumnOptionDef).

Why do you bring that up? Are you planning to work on something where this approach doesn't work well? Can you share the details?


My thoughts on this:

Dialect-specific parsing is required to properly handle cases where grammars of different dialects are in conflict with each other. I can think of two ideas we'll need to combine:

Given that one of the use-cases for the parser is tooling (e.g. @maxcountryman's forma) and analysis, we'll probably want a "generic" parser which is able to parse different dialects of SQL, and run-time switches seem like an easier way to achieve that than build-time ones.

A reason to select a dialect at build time, assuming there already is a run-time switch, as I understand it, is to work with a simpler AST without having to write lots of repetitive code to reject constructs you don't support. Under this assumption it makes more sense for downstream consumers to define their AST types, and for the library to provide a way to do so (I touched on that in #189), rather than providing a limited number of pre-defined dialects as "features".

@miuy56dc
Copy link
Contributor Author

miuy56dc commented Jun 23, 2020

I use sqlparser-rs to parse SQLite's SQL statements. Here is SQLite's CREATE TABLE grammar SQLite CREATE TABLE. It supports WITHOUT ROWID, If I want to implement it for SQLite, I have to add this to Statement::CreateTable. If I do this, others like MySQL, PostgreSQL or MSSQL will implement it too.
And using a dialect of SQLite does not require you to implement WITH.

@nickolay
Copy link
Contributor

sqlite's WITHOUT ROWID is similar to external, location, and file_format, which are already a part of Statement::CreateTable, even though they are Hive-specific. So I think it's fine to add a flag for WITHOUT ROWIDS too.

As I recently noted in an another discussion with @Dandandan, we could change Statement::CreateTable in the generic parser to be a Vec of sub-clauses, such as WITHOUT ROWID, instead of a struct. The motivating factor in that discussion was allowing the sub-clauses to be parsed in any order (and retaining the original order), but it seems relevant here too:

  • It feels less wrong to add dialect-specific things like WITHOUT ROWID to a enum of CREATE TABLE subclauses, than as an optional field to CreateTable. I couldn't come up with an explanation why I feel that though..
  • It may be enough for some use-cases to scan through some clauses without creating new AST types (e.g. when you're interested in the schema, and need other sub-clauses simply not to fail parsing). We could support that with CreateTableClause::DialectSpecific(Vec<Token>)
  • A keyword-based extension approach from Thoughts on SQL Extensions #104 (CreateTableClause::DialectSpecific(Box<dyn ...>), as I understand it) could be experimented with too.

@miuy56dc
Copy link
Contributor Author

miuy56dc commented Jun 24, 2020

I add WITHOUT ROWID to CreateTable like this https://github.com/andygrove/sqlparser-rs/pull/208.
I think both reateTableClause::DialectSpecific(Vec<Token>) and CreateTableClause::DialectSpecific(Box<dyn ...> is good, they do not generate duplicate code.

@AugustoFKL
Copy link
Contributor

@alamb can we close this?

Doesn't seem active for a while.

@alamb
Copy link
Contributor

alamb commented Nov 4, 2022

👍 please reopen if there is more to say (or feel free to file a new issue too)

@alamb alamb closed this as completed Nov 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants