Skip to content

Fixed create snapshot table for bigquery - #2269

Merged
iffyio merged 3 commits into
apache:mainfrom
romanoff:fix_create_snapshot_table_for_bigquery
Mar 13, 2026
Merged

Fixed create snapshot table for bigquery#2269
iffyio merged 3 commits into
apache:mainfrom
romanoff:fix_create_snapshot_table_for_bigquery

Conversation

@romanoff

Copy link
Copy Markdown

Sample query:

CREATE SNAPSHOT TABLE IF NOT EXISTS `dataset_id.table1`
CLONE `dataset_id.table2`;

Spec: https://docs.cloud.google.com/bigquery/docs/reference/standard-sql/data-definition-language#create_snapshot_table_statement

@iffyioiffyio left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @romanoff! The changes looks good to me overall, left some minor comments

self.iceberg = iceberg;
self
}
/// Set `SNAPSHOT` table flag (BigQuery).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Set `SNAPSHOT` table flag (BigQuery).
/// Set `SNAPSHOT` table flag.

Comment threadsrc/ast/helpers/stmt_create_table.rs Outdated
pub volatile: bool,
/// Iceberg-specific table flag.
pub iceberg: bool,
/// BigQuery `SNAPSHOT` table flag.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// BigQuery `SNAPSHOT` table flag.
/// `SNAPSHOT` table flag.

Comment threadsrc/ast/ddl.rs Outdated
pub volatile: bool,
/// `ICEBERG` clause
pub iceberg: bool,
/// BigQuery `SNAPSHOT` clause

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// BigQuery `SNAPSHOT` clause
/// `SNAPSHOT` clause

Comment threadsrc/parser/mod.rs Outdated
&& 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]) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ifself.parse_keywords(&[Keyword::SNAPSHOT,Keyword::TABLE]){
ifself.peek_keywords(&[Keyword::SNAPSHOT,Keyword::TABLE]){

thinking we can peek here so that the parse_create_snapshot_table() function is standalone

Comment threadsrc/parser/mod.rs Outdated
.build())
}

/// Parse BigQuery `CREATE SNAPSHOT TABLE` statement.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Parse BigQuery `CREATE SNAPSHOT TABLE` statement.
/// Parse `CREATE SNAPSHOT TABLE` statement.

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

Comment threadtests/sqlparser_bigquery.rs Outdated

#[test]
fn test_create_snapshot_table() {
bigquery().verified_stmt("CREATE SNAPSHOT TABLE dataset_id.table1 CLONE dataset_id.table2");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
bigquery().verified_stmt("CREATE SNAPSHOT TABLE dataset_id.table1 CLONE dataset_id.table2");
bigquery_and_generic().verified_stmt("CREATE SNAPSHOT TABLE dataset_id.table1 CLONE dataset_id.table2");

can the tests cover generic as well?

@romanoff
romanoffforce-pushed the fix_create_snapshot_table_for_bigquery branch from 87e38a1 to 53944f4CompareMarch 9, 2026 23:43
@romanoff

Copy link
Copy Markdown
Author

Thanks @iffyio. Updated

@iffyioiffyio left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks @romanoff!

@iffyio
iffyio added this pull request to the merge queueMar 13, 2026
Merged via the queue into apache:main with commit 738f12dMar 13, 2026
10 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@romanoff@iffyio