From f2e3ea5f7f28efb22e06ee0ea29a9f7c039c67ae Mon Sep 17 00:00:00 2001 From: achristmascarl Date: Tue, 15 Jul 2025 11:15:35 -0400 Subject: [PATCH 1/5] parse SET ( storage_parameters ) --- src/ast/ddl.rs | 7 +++++++ src/ast/spans.rs | 3 +++ src/parser/mod.rs | 22 ++++++++++++++-------- 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/ast/ddl.rs b/src/ast/ddl.rs index 51e057840..8e4addebb 100644 --- a/src/ast/ddl.rs +++ b/src/ast/ddl.rs @@ -351,6 +351,10 @@ pub enum AlterTableOperation { ValidateConstraint { name: Ident, }, + /// `SET ( storage_parameter [= value] [, ... ] )` + SetStorageParameters { + storage_parameters: Vec, + }, } /// An `ALTER Policy` (`Statement::AlterPolicy`) operation @@ -791,6 +795,9 @@ impl fmt::Display for AlterTableOperation { AlterTableOperation::ValidateConstraint { name } => { write!(f, "VALIDATE CONSTRAINT {name}") } + AlterTableOperation::SetStorageParameters { storage_parameters } => { + write!(f, "SET ({})", display_comma_separated(storage_parameters)) + } } } } diff --git a/src/ast/spans.rs b/src/ast/spans.rs index 3e82905e1..f058705b6 100644 --- a/src/ast/spans.rs +++ b/src/ast/spans.rs @@ -1201,6 +1201,9 @@ impl Spanned for AlterTableOperation { AlterTableOperation::Lock { .. } => Span::empty(), AlterTableOperation::ReplicaIdentity { .. } => Span::empty(), AlterTableOperation::ValidateConstraint { name } => name.span, + AlterTableOperation::SetStorageParameters { storage_parameters } => { + union_spans(storage_parameters.iter().map(|i| i.span())) + } } } } diff --git a/src/parser/mod.rs b/src/parser/mod.rs index 3bb913118..b91ad7833 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -8938,17 +8938,23 @@ impl<'a> Parser<'a> { let name = self.parse_identifier()?; AlterTableOperation::ValidateConstraint { name } } else { - let options: Vec = - self.parse_options_with_keywords(&[Keyword::SET, Keyword::TBLPROPERTIES])?; - if !options.is_empty() { - AlterTableOperation::SetTblProperties { - table_properties: options, + let maybe_options = self.maybe_parse_options(Keyword::SET)?; + if let Some(options) = maybe_options { + AlterTableOperation::SetStorageParameters { + storage_parameters: options, } } else { - return self.expected( - "ADD, RENAME, PARTITION, SWAP, DROP, REPLICA IDENTITY, or SET TBLPROPERTIES after ALTER TABLE", + let options: Vec = self.parse_options(Keyword::TBLPROPERTIES)?; + if !options.is_empty() { + AlterTableOperation::SetTblProperties { + table_properties: options, + } + } else { + return self.expected( + "ADD, RENAME, PARTITION, SWAP, DROP, REPLICA IDENTITY, SET, or SET TBLPROPERTIES after ALTER TABLE", self.peek_token(), - ); + ); + } } }; Ok(operation) From 7df32ea908dc863e7a524addb93ff940866b96bc Mon Sep 17 00:00:00 2001 From: achristmascarl Date: Tue, 15 Jul 2025 11:30:46 -0400 Subject: [PATCH 2/5] add test --- tests/sqlparser_common.rs | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index 4183c5539..12619e505 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -4725,6 +4725,34 @@ fn parse_alter_table() { } _ => unreachable!(), } + + let set_storage_parameters = "ALTER TABLE tab SET (autovacuum_vacuum_scale_factor = 0.01, autovacuum_vacuum_threshold = 500)"; + match alter_table_op(verified_stmt(set_storage_parameters)) { + AlterTableOperation::SetStorageParameters { storage_parameters } => { + assert_eq!( + storage_parameters, + [ + SqlOption::KeyValue { + key: Ident { + value: "autovacuum_vacuum_scale_factor".to_string(), + quote_style: None, + span: Span::empty(), + }, + value: Expr::Value(test_utils::number("0.01").with_empty_span()), + }, + SqlOption::KeyValue { + key: Ident { + value: "autovacuum_vacuum_threshold".to_string(), + quote_style: None, + span: Span::empty(), + }, + value: Expr::Value(test_utils::number("500").with_empty_span()), + } + ], + ); + } + _ => unreachable!(), + } } #[test] From d76a9c9623734231e37bfea083dfdc4fbe6746f7 Mon Sep 17 00:00:00 2001 From: achristmascarl Date: Tue, 15 Jul 2025 11:36:13 -0400 Subject: [PATCH 3/5] fix TBLPROPERTIES parsing --- src/parser/mod.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/parser/mod.rs b/src/parser/mod.rs index b91ad7833..210e79953 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -8938,16 +8938,17 @@ impl<'a> Parser<'a> { let name = self.parse_identifier()?; AlterTableOperation::ValidateConstraint { name } } else { - let maybe_options = self.maybe_parse_options(Keyword::SET)?; - if let Some(options) = maybe_options { - AlterTableOperation::SetStorageParameters { - storage_parameters: options, + let mut options = + self.parse_options_with_keywords(&[Keyword::SET, Keyword::TBLPROPERTIES])?; + if !options.is_empty() { + AlterTableOperation::SetTblProperties { + table_properties: options, } } else { - let options: Vec = self.parse_options(Keyword::TBLPROPERTIES)?; + options = self.parse_options(Keyword::SET)?; if !options.is_empty() { - AlterTableOperation::SetTblProperties { - table_properties: options, + AlterTableOperation::SetStorageParameters { + storage_parameters: options, } } else { return self.expected( From 8532dd37c8f98792ef3b54a0beedb15aa388ba5a Mon Sep 17 00:00:00 2001 From: achristmascarl Date: Fri, 18 Jul 2025 10:54:34 -0400 Subject: [PATCH 4/5] pr feedback: generic naming for syntax --- src/ast/ddl.rs | 11 +++++++---- src/ast/spans.rs | 4 ++-- src/parser/mod.rs | 4 +--- tests/sqlparser_common.rs | 4 ++-- 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/ast/ddl.rs b/src/ast/ddl.rs index 8e4addebb..28d4cd95b 100644 --- a/src/ast/ddl.rs +++ b/src/ast/ddl.rs @@ -352,8 +352,11 @@ pub enum AlterTableOperation { name: Ident, }, /// `SET ( storage_parameter [= value] [, ... ] )` - SetStorageParameters { - storage_parameters: Vec, + /// + /// Note: this is a PostgreSQL-specific operation. + /// Please refer to [PostgreSQL documentation](https://www.postgresql.org/docs/current/sql-altertable.html) + SetOptionsParens { + options: Vec, }, } @@ -795,8 +798,8 @@ impl fmt::Display for AlterTableOperation { AlterTableOperation::ValidateConstraint { name } => { write!(f, "VALIDATE CONSTRAINT {name}") } - AlterTableOperation::SetStorageParameters { storage_parameters } => { - write!(f, "SET ({})", display_comma_separated(storage_parameters)) + AlterTableOperation::SetOptionsParens { options } => { + write!(f, "SET ({})", display_comma_separated(options)) } } } diff --git a/src/ast/spans.rs b/src/ast/spans.rs index f058705b6..cb348a0e5 100644 --- a/src/ast/spans.rs +++ b/src/ast/spans.rs @@ -1201,8 +1201,8 @@ impl Spanned for AlterTableOperation { AlterTableOperation::Lock { .. } => Span::empty(), AlterTableOperation::ReplicaIdentity { .. } => Span::empty(), AlterTableOperation::ValidateConstraint { name } => name.span, - AlterTableOperation::SetStorageParameters { storage_parameters } => { - union_spans(storage_parameters.iter().map(|i| i.span())) + AlterTableOperation::SetOptionsParens { options } => { + union_spans(options.iter().map(|i| i.span())) } } } diff --git a/src/parser/mod.rs b/src/parser/mod.rs index 210e79953..705875eaa 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -8947,9 +8947,7 @@ impl<'a> Parser<'a> { } else { options = self.parse_options(Keyword::SET)?; if !options.is_empty() { - AlterTableOperation::SetStorageParameters { - storage_parameters: options, - } + AlterTableOperation::SetOptionsParens { options } } else { return self.expected( "ADD, RENAME, PARTITION, SWAP, DROP, REPLICA IDENTITY, SET, or SET TBLPROPERTIES after ALTER TABLE", diff --git a/tests/sqlparser_common.rs b/tests/sqlparser_common.rs index 12619e505..b9b0d2f5e 100644 --- a/tests/sqlparser_common.rs +++ b/tests/sqlparser_common.rs @@ -4728,9 +4728,9 @@ fn parse_alter_table() { let set_storage_parameters = "ALTER TABLE tab SET (autovacuum_vacuum_scale_factor = 0.01, autovacuum_vacuum_threshold = 500)"; match alter_table_op(verified_stmt(set_storage_parameters)) { - AlterTableOperation::SetStorageParameters { storage_parameters } => { + AlterTableOperation::SetOptionsParens { options } => { assert_eq!( - storage_parameters, + options, [ SqlOption::KeyValue { key: Ident { From 9ae0afb47cf17ef4adb678d1d8873dc943969176 Mon Sep 17 00:00:00 2001 From: carl <44021312+achristmascarl@users.noreply.github.com> Date: Mon, 21 Jul 2025 10:00:39 -0400 Subject: [PATCH 5/5] update comment docs Co-authored-by: Ifeanyi Ubah --- src/ast/ddl.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/ast/ddl.rs b/src/ast/ddl.rs index 28d4cd95b..92936a6f0 100644 --- a/src/ast/ddl.rs +++ b/src/ast/ddl.rs @@ -351,10 +351,13 @@ pub enum AlterTableOperation { ValidateConstraint { name: Ident, }, - /// `SET ( storage_parameter [= value] [, ... ] )` + /// Arbitrary parenthesized `SET` options. /// - /// Note: this is a PostgreSQL-specific operation. - /// Please refer to [PostgreSQL documentation](https://www.postgresql.org/docs/current/sql-altertable.html) + /// Example: + /// ```sql + /// SET (scale_factor = 0.01, threshold = 500)` + /// ``` + /// [PostgreSQL](https://www.postgresql.org/docs/current/sql-altertable.html) SetOptionsParens { options: Vec, },