-
Notifications
You must be signed in to change notification settings - Fork 711
Fixed create snapshot table for bigquery #2269
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -81,6 +81,8 @@ pub struct CreateTableBuilder { | |||||
| pub volatile: bool, | ||||||
| /// Iceberg-specific table flag. | ||||||
| pub iceberg: bool, | ||||||
| /// BigQuery `SNAPSHOT` table flag. | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| pub snapshot: bool, | ||||||
| /// Whether `DYNAMIC` table option is set. | ||||||
| pub dynamic: bool, | ||||||
| /// The table name. | ||||||
|
|
@@ -189,6 +191,7 @@ impl CreateTableBuilder { | |||||
| transient: false, | ||||||
| volatile: false, | ||||||
| iceberg: false, | ||||||
| snapshot: false, | ||||||
| dynamic: false, | ||||||
| name, | ||||||
| columns: vec![], | ||||||
|
|
@@ -278,6 +281,11 @@ impl CreateTableBuilder { | |||||
| self.iceberg = iceberg; | ||||||
| self | ||||||
| } | ||||||
| /// Set `SNAPSHOT` table flag (BigQuery). | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| pub fn snapshot(mut self, snapshot: bool) -> Self { | ||||||
| self.snapshot = snapshot; | ||||||
| self | ||||||
| } | ||||||
| /// Set `DYNAMIC` table option. | ||||||
| pub fn dynamic(mut self, dynamic: bool) -> Self { | ||||||
| self.dynamic = dynamic; | ||||||
|
|
@@ -532,6 +540,7 @@ impl CreateTableBuilder { | |||||
| transient: self.transient, | ||||||
| volatile: self.volatile, | ||||||
| iceberg: self.iceberg, | ||||||
| snapshot: self.snapshot, | ||||||
| dynamic: self.dynamic, | ||||||
| name: self.name, | ||||||
| columns: self.columns, | ||||||
|
|
@@ -609,6 +618,7 @@ impl From<CreateTable> for CreateTableBuilder { | |||||
| transient: table.transient, | ||||||
| volatile: table.volatile, | ||||||
| iceberg: table.iceberg, | ||||||
| snapshot: table.snapshot, | ||||||
| dynamic: table.dynamic, | ||||||
| name: table.name, | ||||||
| columns: table.columns, | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -5099,7 +5099,9 @@ impl<'a> Parser<'a> { | |||||
| let persistent = dialect_of!(self is DuckDbDialect) | ||||||
| && self.parse_one_of_keywords(&[Keyword::PERSISTENT]).is_some(); | ||||||
| let create_view_params = self.parse_create_view_params()?; | ||||||
| if self.parse_keyword(Keyword::TABLE) { | ||||||
| if self.parse_keywords(&[Keyword::SNAPSHOT, Keyword::TABLE]) { | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
thinking we can peek here so that the |
||||||
| self.parse_create_snapshot_table().map(Into::into) | ||||||
| } else if self.parse_keyword(Keyword::TABLE) { | ||||||
| self.parse_create_table(or_replace, temporary, global, transient) | ||||||
| .map(Into::into) | ||||||
| } else if self.peek_keyword(Keyword::MATERIALIZED) | ||||||
|
|
@@ -6314,6 +6316,39 @@ impl<'a> Parser<'a> { | |||||
| .build()) | ||||||
| } | ||||||
|
|
||||||
| /// Parse BigQuery `CREATE SNAPSHOT TABLE` statement. | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
I'm thinking in general we can avoid inlining the dialect in the description (the doc links provide context on which dialects support the syntax). Otherwise it becomes confusing/misleading over time documentation wise as multiple dialects support the same or part of the syntax |
||||||
| /// | ||||||
| /// <https://cloud.google.com/bigquery/docs/reference/standard-sql/data-definition-language#create_snapshot_table_statement> | ||||||
| pub fn parse_create_snapshot_table(&mut self) -> Result<CreateTable, ParserError> { | ||||||
| let if_not_exists = self.parse_keywords(&[Keyword::IF, Keyword::NOT, Keyword::EXISTS]); | ||||||
| let table_name = self.parse_object_name(true)?; | ||||||
|
|
||||||
| self.expect_keyword_is(Keyword::CLONE)?; | ||||||
| let clone = Some(self.parse_object_name(true)?); | ||||||
|
|
||||||
| let version = | ||||||
| if self.parse_keywords(&[Keyword::FOR, Keyword::SYSTEM_TIME, Keyword::AS, Keyword::OF]) | ||||||
| { | ||||||
| Some(TableVersion::ForSystemTimeAsOf(self.parse_expr()?)) | ||||||
| } else { | ||||||
| None | ||||||
| }; | ||||||
|
|
||||||
| let table_options = if let Some(options) = self.maybe_parse_options(Keyword::OPTIONS)? { | ||||||
| CreateTableOptions::Options(options) | ||||||
| } else { | ||||||
| CreateTableOptions::None | ||||||
| }; | ||||||
|
|
||||||
| Ok(CreateTableBuilder::new(table_name) | ||||||
| .snapshot(true) | ||||||
| .if_not_exists(if_not_exists) | ||||||
| .clone_clause(clone) | ||||||
| .version(version) | ||||||
| .table_options(table_options) | ||||||
| .build()) | ||||||
| } | ||||||
|
|
||||||
| /// Parse a file format for external tables. | ||||||
| pub fn parse_file_format(&mut self) -> Result<FileFormat, ParserError> { | ||||||
| let next_token = self.next_token(); | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -2890,3 +2890,24 @@ fn test_alter_schema() { | |||||
| bigquery_and_generic() | ||||||
| .verified_stmt("ALTER SCHEMA IF EXISTS mydataset SET OPTIONS (location = 'us')"); | ||||||
| } | ||||||
|
|
||||||
| #[test] | ||||||
| fn test_create_snapshot_table() { | ||||||
| bigquery().verified_stmt("CREATE SNAPSHOT TABLE dataset_id.table1 CLONE dataset_id.table2"); | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
can the tests cover generic as well? |
||||||
|
|
||||||
| bigquery().verified_stmt( | ||||||
| "CREATE SNAPSHOT TABLE IF NOT EXISTS dataset_id.table1 CLONE dataset_id.table2", | ||||||
| ); | ||||||
|
|
||||||
| bigquery().verified_stmt( | ||||||
| "CREATE SNAPSHOT TABLE dataset_id.table1 CLONE dataset_id.table2 FOR SYSTEM_TIME AS OF TIMESTAMP_SUB(CURRENT_TIMESTAMP(), INTERVAL 1 HOUR)", | ||||||
| ); | ||||||
|
|
||||||
| bigquery().verified_stmt( | ||||||
| "CREATE SNAPSHOT TABLE dataset_id.table1 CLONE dataset_id.table2 OPTIONS(expiration_timestamp = TIMESTAMP '2025-01-01 00:00:00 UTC', friendly_name = 'my_table')", | ||||||
| ); | ||||||
|
|
||||||
| bigquery().verified_stmt( | ||||||
| "CREATE SNAPSHOT TABLE IF NOT EXISTS dataset_id.table1 CLONE dataset_id.table2 FOR SYSTEM_TIME AS OF TIMESTAMP_SUB(CURRENT_TIMESTAMP(), INTERVAL 1 HOUR) OPTIONS(expiration_timestamp = TIMESTAMP '2025-01-01 00:00:00 UTC')", | ||||||
| ); | ||||||
| } | ||||||
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.