-
Notifications
You must be signed in to change notification settings - Fork 670
Deferrable column option #470
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
Conversation
* Add support in the position function * Add Test * Add lint fixes * Remove if * Change from to in * Remove special method for position * Fix lint * PR Review
1e64864 to
89804fd
Compare
alamb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @graham -- thank you for the contribution! I am sorry I did not see this before. i left some suggestions for your consideration
| field: DateTimeField, | ||
| expr: Box<Expr>, | ||
| }, | ||
| /// POSITION(<expr> in <expr>) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this change appears to be unrelated to DEFERRABLE INITIALLY DEFERRED
I think it was already added in #463
| DialectSpecific(Vec<Token>), | ||
| CharacterSet(ObjectName), | ||
| Comment(String), | ||
| DeferrableInitiallyDeferred, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a docstring noting this is from SQLite?
The reference manual https://www.sqlite.org/syntax/foreign-key-clause.html seems to show that it is possible to have NOT INITIALLY DEFERRED as well
What if this was represented with something like
/// SQlite foreign key deferrability
/// https://www.sqlite.org/syntax/foreign-key-clause.html
pub enum DeferType {
Deferred,
Immediate
}
pub enum ColumnOption {
...
/// Sqlite constraint deferrability
/// https://www.sqlite.org/syntax/foreign-key-clause.html
Deferrable {
not: bool,
defer_type: DeferType,
}
...|
Marking this as a draft to show it is waiting some changes so I can easily filter out which PRs need review. Please mark it as "ready for review" when it is next ready. |
|
Closing as stale -- please reopen if you still plan to work on it |
While using sqlparser-rs to parse schema from a Django SQLite database I found that deferrable wasn't handled.
Since it exists here: https://jakewheat.github.io/sql-overview/sql-2016-foundation-grammar.html#_10_8_constraint_name_definition_and_constraint_characteristics I assume it's reasonable for inclusion.
Happy to make changes to the PR as required, relatively new to rust so I'm well aware I may have style/common practice issues.