97nitt commented on code in PR #2326:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/2326#discussion_r3162167704
##########
src/ast/helpers/stmt_create_table.rs:
##########
@@ -157,6 +157,9 @@ pub struct CreateTableBuilder {
pub base_location: Option<String>,
/// Optional external volume identifier.
pub external_volume: Option<String>,
+ /// BigQuery `WITH CONNECTION` clause for external tables.
+ ///
<https://cloud.google.com/bigquery/docs/reference/standard-sql/data-definition-language#create_external_table_statement>
Review Comment:
Done.
##########
src/ast/helpers/stmt_create_table.rs:
##########
@@ -488,6 +492,11 @@ impl CreateTableBuilder {
self.external_volume = external_volume;
self
}
+ /// Set the BigQuery `WITH CONNECTION` clause for external tables.
Review Comment:
Done.
##########
src/ast/ddl.rs:
##########
@@ -3021,6 +3021,9 @@ pub struct CreateTable {
/// Snowflake "EXTERNAL_VOLUME" clause for Iceberg tables
/// <https://docs.snowflake.com/en/sql-reference/sql/create-iceberg-table>
pub external_volume: Option<String>,
+ /// BigQuery `WITH CONNECTION` clause for external tables
+ ///
<https://cloud.google.com/bigquery/docs/reference/standard-sql/data-definition-language#create_external_table_statement>
Review Comment:
Done.
##########
src/parser/mod.rs:
##########
@@ -6416,8 +6416,28 @@ impl<'a> Parser<'a> {
None
};
let location = hive_formats.as_ref().and_then(|hf|
hf.location.clone());
- let table_properties = self.parse_options(Keyword::TBLPROPERTIES)?;
- let table_options = if !table_properties.is_empty() {
+
+ // BigQuery external tables support `WITH CONNECTION <name>` and
`OPTIONS(...)`
+ // clauses after the (optional) column list.
Review Comment:
Done.
##########
src/parser/mod.rs:
##########
@@ -6416,8 +6416,28 @@ impl<'a> Parser<'a> {
None
};
let location = hive_formats.as_ref().and_then(|hf|
hf.location.clone());
- let table_properties = self.parse_options(Keyword::TBLPROPERTIES)?;
- let table_options = if !table_properties.is_empty() {
+
+ // BigQuery external tables support `WITH CONNECTION <name>` and
`OPTIONS(...)`
+ // clauses after the (optional) column list.
+ let with_connection = if self.parse_keywords(&[Keyword::WITH,
Keyword::CONNECTION]) {
+ Some(self.parse_object_name(false)?)
+ } else {
+ None
+ };
+ // BigQuery uses OPTIONS(...); Hive uses TBLPROPERTIES(...). They are
+ // mutually exclusive in practice, and `parse_options` returns an empty
+ // vec when the keyword isn't present, so trying OPTIONS first and
+ // falling back to TBLPROPERTIES preserves the existing Hive path
+ // without accepting both in the same statement.
+ let options = self.parse_options(Keyword::OPTIONS)?;
Review Comment:
Good catch, reverted. The parser change is now just the new `WITH
CONNECTION` block plus the `.with_connection(...)` builder line; the fallback
chain handles `OPTIONS` unchanged.
##########
tests/sqlparser_bigquery.rs:
##########
@@ -2203,6 +2203,90 @@ fn parse_big_query_declare() {
);
}
+#[test]
+fn parse_bigquery_create_external_table_with_connection_and_options() {
+ let sql = concat!(
+ "CREATE OR REPLACE EXTERNAL TABLE `proj.ds.tbl` ",
+ "WITH CONNECTION `projects/proj/locations/us/connections/c` ",
+ r#"OPTIONS(format = "ICEBERG", uris = ["gs://b/m.json"])"#,
+ );
+ let stmts = bigquery().parse_sql_statements(sql).unwrap();
+ assert_eq!(stmts.len(), 1);
+ let Statement::CreateTable(ct) = &stmts[0] else {
+ panic!("expected CreateTable, got {:?}", stmts[0]);
+ };
+ assert!(ct.external);
+ assert!(ct.or_replace);
+ assert!(ct.columns.is_empty());
+ let with_connection = ct.with_connection.as_ref().expect("with_connection
set");
+ assert_eq!(
+ with_connection.to_string(),
+ "`projects/proj/locations/us/connections/c`"
+ );
+ let CreateTableOptions::Options(opts) = &ct.table_options else {
+ panic!("expected OPTIONS, got {:?}", ct.table_options);
+ };
+ assert_eq!(opts.len(), 2);
+}
+
+#[test]
+fn parse_bigquery_create_external_table_with_connection_variants() {
+ // WITH CONNECTION alone — no OPTIONS, no explicit column list.
+ // Display normalizes the bare form by adding an empty column list, so
+ // use `one_statement_parses_to` to assert the normalized output.
+ let stmt = bigquery().one_statement_parses_to(
+ "CREATE EXTERNAL TABLE t WITH CONNECTION c",
+ "CREATE EXTERNAL TABLE t () WITH CONNECTION c",
+ );
+ let Statement::CreateTable(ct) = stmt else {
+ panic!("expected CreateTable");
+ };
+ assert!(ct.external);
+ assert!(!ct.or_replace);
+ assert!(ct.columns.is_empty());
+ assert_eq!(
+ ct.with_connection
+ .as_ref()
+ .expect("with_connection set")
+ .to_string(),
+ "c"
+ );
+ assert!(matches!(ct.table_options, CreateTableOptions::None));
+
+ // With explicit columns + OPTIONS. Exercises the Display round-trip of
+ // the `(columns) WITH CONNECTION <name> OPTIONS(...)` ordering so a
+ // future refactor that reshuffles clause order will fail this test.
+ let sql = concat!(
+ "CREATE EXTERNAL TABLE t (a INT64, b STRING) ",
+ r#"WITH CONNECTION c OPTIONS(uris = ["gs://x"])"#,
+ );
+ let stmt = bigquery().verified_stmt(sql);
+ let Statement::CreateTable(ct) = stmt else {
+ panic!("expected CreateTable");
+ };
+ assert_eq!(ct.columns.len(), 2);
+ assert_eq!(
+ ct.with_connection
+ .as_ref()
+ .expect("with_connection set")
+ .to_string(),
+ "c"
+ );
+ let CreateTableOptions::Options(opts) = &ct.table_options else {
+ panic!("expected OPTIONS, got {:?}", ct.table_options);
+ };
+ assert_eq!(opts.len(), 1);
+
+ // Baseline: no WITH CONNECTION. The new parser branch must not produce
Review Comment:
Done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
