-
Notifications
You must be signed in to change notification settings - Fork 625
Snowflake: support trailing options in CREATE TABLE
#1931
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
src/ast/helpers/stmt_create_table.rs
Outdated
/// Returns true if information on the structure of the table | ||
/// to be created was provided to the builder. If not, the | ||
/// statement is invalid. | ||
pub fn has_schema_info(&self) -> bool { |
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.
pub fn has_schema_info(&self) -> bool { | |
pub(crate) fn has_schema_info(&self) -> bool { |
src/ast/helpers/stmt_create_table.rs
Outdated
!self.columns.is_empty() | ||
|| self.query.is_some() | ||
|| self.like.is_some() | ||
|| self.clone.is_some() |
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.
could we add tests covering this behavior? e.g a test that fails when only a query, columns, like etc is provided in a create statement
tests/sqlparser_snowflake.rs
Outdated
snowflake() | ||
.parse_sql_statements( | ||
"CREATE TEMP TABLE dst AS (SELECT * FROM src) ON COMMIT PRESERVE ROWS", | ||
) | ||
.unwrap(); | ||
snowflake() | ||
.parse_sql_statements("CREATE TEMP TABLE tbl LIKE customers ON COMMIT PRESERVE ROWS;") | ||
.unwrap(); | ||
snowflake() | ||
.parse_sql_statements("CREATE TEMP TABLE tbl CLONE customers ON COMMIT PRESERVE ROWS;") | ||
.unwrap(); |
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 use verified_stmt here or similar roundtrip assertion?
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.
Improved the tests, note Snowflake supports specifying the options either before or after the "source" definition while the SQL output assumes one. Added tests for both options.
CREATE TABLE
dd8ad08
to
794cdcf
Compare
c21ea51
to
3aa41f3
Compare
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.
LGTM! Thanks @yoavcloud!
cc @alamb
The code that parses
CREATE TABLE
in the Snowflake dialect assumed that if theAS
,LIKE
orCLONE
options are used, then no other options can be specified. That is not true, so that restriction is now removed.For example:
CREATE TEMP TABLE dst AS (SELECT * FROM src) ON COMMIT PRESERVE ROWS