-
Notifications
You must be signed in to change notification settings - Fork 561
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
Handle CREATE [TEMPORARY|TEMP] VIEW [IF NOT EXISTS] #993
Conversation
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.
Looks good to me -- thank you very much @gabivlj 🙏
Pull Request Test Coverage Report for Build 6423607217
💛 - Coveralls |
The CI failures do not appear to be related to this PR -- see #995 |
@gabivlj if you can merge up from main, I think the CI will pass on this PR. I tried to do it but I do not have permissions:
|
Sure! Thanks for the heads up |
Thanks again @gabivlj |
thanks ! |
let materialized = self.parse_keyword(Keyword::MATERIALIZED); | ||
self.expect_keyword(Keyword::VIEW)?; | ||
let if_not_exists = dialect_of!(self is SQLiteDialect|GenericDialect) |
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.
Is this on purpose ? Most other constructs do not generate parse errors when parsed with the "wrong" dialect. Is there a rule for how sqlparser handles that, or does every contributor do it their own way ?
I was under the impression that dialects allowed disambiguating syntax that would parse differently in different databases, but did not cause a parse error when parsing unambiguous syntax from one dialect with a different one.
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.
For context, I took the idea from already existing RedshiftSqlDialect with_no_schema_binding
. The intent here was excluding Dialects that do not support CREATE VIEW IF NOT EXISTS
, for example the most similar feature for Postgresql is CREATE OR REPLACE VIEW
as IF NOT EXISTS
on a view is not supported there.
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.
Yes, I see that. There does not seem to be a coherent rule over the entire codebase for whether database-specific syntax should cause a syntax error when parsed with a different dialect.
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.
Yes, I see that. There does not seem to be a coherent rule over the entire codebase for whether database-specific syntax should cause a syntax error when parsed with a different dialect.
I agree this is not uniform in the codebase and I think sqlparser-rs is more permissive than most other parsers in the sense that it will parse SQL that would generate a syntax error in the original implementation. This is broadly consistent with the sentiment of https://github.com/sqlparser-rs/sqlparser-rs#syntax-vs-semantics
In general I expect that the GenericDialect
will parse the union of all other dialects, as long as they aren't in conflict with each other (e.g. the rules on how to parse strings vs identifiers differ in some dialects)
Tried giving this a go as I encountered the same problem as #976! Let me know what you folks think or if there is any changes needed!
Closes #976