From 57625fa38e212f3796eb78233a2876a7c7e2c2f9 Mon Sep 17 00:00:00 2001 From: Filipe Guerreiro Date: Fri, 17 Apr 2026 11:24:05 +0900 Subject: [PATCH 01/17] feat(ast): add CreateAggregate type Add Statement::CreateAggregate, CreateAggregate struct, CreateAggregateOption enum, and AggregateModifyKind enum to represent PostgreSQL CREATE AGGREGATE DDL. Options are stored as a typed enum covering all documented parameters (SFUNC, STYPE, FINALFUNC, PARALLEL, moving-aggregate variants, etc.). --- src/ast/ddl.rs | 153 +++++++++++++++++++++++++++++++++++++++++++++++++ src/ast/mod.rs | 15 +++-- 2 files changed, 164 insertions(+), 4 deletions(-) diff --git a/src/ast/ddl.rs b/src/ast/ddl.rs index 67aefb3928..4f511dfb95 100644 --- a/src/ast/ddl.rs +++ b/src/ast/ddl.rs @@ -5757,3 +5757,156 @@ impl From for crate::ast::Statement { crate::ast::Statement::AlterPolicy(v) } } + +/// CREATE AGGREGATE statement. +/// See +#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)] +#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] +#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))] +pub struct CreateAggregate { + /// True if `OR REPLACE` was specified. + pub or_replace: bool, + /// The aggregate name (can be schema-qualified). + pub name: ObjectName, + /// Input argument types. Empty for zero-argument aggregates. + pub args: Vec, + /// The options listed inside the required parentheses after the argument + /// list (e.g. `SFUNC`, `STYPE`, `FINALFUNC`, `PARALLEL`, …). + pub options: Vec, +} + +impl fmt::Display for CreateAggregate { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "CREATE")?; + if self.or_replace { + write!(f, " OR REPLACE")?; + } + write!(f, " AGGREGATE {}", self.name)?; + write!(f, " ({})", display_comma_separated(&self.args))?; + write!(f, " (")?; + for (i, option) in self.options.iter().enumerate() { + if i > 0 { + write!(f, ", ")?; + } + write!(f, "{option}")?; + } + write!(f, ")") + } +} + +impl From for crate::ast::Statement { + fn from(v: CreateAggregate) -> Self { + crate::ast::Statement::CreateAggregate(v) + } +} + +/// A single option in a `CREATE AGGREGATE` options list. +/// +/// See +#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)] +#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] +#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))] +pub enum CreateAggregateOption { + /// `SFUNC = state_transition_function` + Sfunc(ObjectName), + /// `STYPE = state_data_type` + Stype(DataType), + /// `SSPACE = state_data_size` (in bytes) + Sspace(u64), + /// `FINALFUNC = final_function` + Finalfunc(ObjectName), + /// `FINALFUNC_EXTRA` — pass extra dummy arguments to the final function. + FinalfuncExtra, + /// `FINALFUNC_MODIFY = { READ_ONLY | SHAREABLE | READ_WRITE }` + FinalfuncModify(AggregateModifyKind), + /// `COMBINEFUNC = combine_function` + Combinefunc(ObjectName), + /// `SERIALFUNC = serial_function` + Serialfunc(ObjectName), + /// `DESERIALFUNC = deserial_function` + Deserialfunc(ObjectName), + /// `INITCOND = initial_condition` (a string literal) + Initcond(Value), + /// `MSFUNC = moving_state_transition_function` + Msfunc(ObjectName), + /// `MINVFUNC = moving_inverse_transition_function` + Minvfunc(ObjectName), + /// `MSTYPE = moving_state_data_type` + Mstype(DataType), + /// `MSSPACE = moving_state_data_size` (in bytes) + Msspace(u64), + /// `MFINALFUNC = moving_final_function` + Mfinalfunc(ObjectName), + /// `MFINALFUNC_EXTRA` + MfinalfuncExtra, + /// `MFINALFUNC_MODIFY = { READ_ONLY | SHAREABLE | READ_WRITE }` + MfinalfuncModify(AggregateModifyKind), + /// `MINITCOND = moving_initial_condition` (a string literal) + Minitcond(Value), + /// `SORTOP = sort_operator` + Sortop(ObjectName), + /// `PARALLEL = { SAFE | RESTRICTED | UNSAFE }` + Parallel(FunctionParallel), + /// `HYPOTHETICAL` — marks the aggregate as hypothetical-set. + Hypothetical, +} + +impl fmt::Display for CreateAggregateOption { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + Self::Sfunc(name) => write!(f, "SFUNC = {name}"), + Self::Stype(data_type) => write!(f, "STYPE = {data_type}"), + Self::Sspace(size) => write!(f, "SSPACE = {size}"), + Self::Finalfunc(name) => write!(f, "FINALFUNC = {name}"), + Self::FinalfuncExtra => write!(f, "FINALFUNC_EXTRA"), + Self::FinalfuncModify(kind) => write!(f, "FINALFUNC_MODIFY = {kind}"), + Self::Combinefunc(name) => write!(f, "COMBINEFUNC = {name}"), + Self::Serialfunc(name) => write!(f, "SERIALFUNC = {name}"), + Self::Deserialfunc(name) => write!(f, "DESERIALFUNC = {name}"), + Self::Initcond(cond) => write!(f, "INITCOND = {cond}"), + Self::Msfunc(name) => write!(f, "MSFUNC = {name}"), + Self::Minvfunc(name) => write!(f, "MINVFUNC = {name}"), + Self::Mstype(data_type) => write!(f, "MSTYPE = {data_type}"), + Self::Msspace(size) => write!(f, "MSSPACE = {size}"), + Self::Mfinalfunc(name) => write!(f, "MFINALFUNC = {name}"), + Self::MfinalfuncExtra => write!(f, "MFINALFUNC_EXTRA"), + Self::MfinalfuncModify(kind) => write!(f, "MFINALFUNC_MODIFY = {kind}"), + Self::Minitcond(cond) => write!(f, "MINITCOND = {cond}"), + Self::Sortop(name) => write!(f, "SORTOP = {name}"), + Self::Parallel(parallel) => { + let kind = match parallel { + FunctionParallel::Safe => "SAFE", + FunctionParallel::Restricted => "RESTRICTED", + FunctionParallel::Unsafe => "UNSAFE", + }; + write!(f, "PARALLEL = {kind}") + } + Self::Hypothetical => write!(f, "HYPOTHETICAL"), + } + } +} + +/// Modifier kind for `FINALFUNC_MODIFY` / `MFINALFUNC_MODIFY` in `CREATE AGGREGATE`. +/// +/// See +#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)] +#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] +#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))] +pub enum AggregateModifyKind { + /// The final function does not modify the transition state. + ReadOnly, + /// The transition state may be shared between aggregate calls. + Shareable, + /// The final function may modify the transition state. + ReadWrite, +} + +impl fmt::Display for AggregateModifyKind { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + Self::ReadOnly => write!(f, "READ_ONLY"), + Self::Shareable => write!(f, "SHAREABLE"), + Self::ReadWrite => write!(f, "READ_WRITE"), + } + } +} diff --git a/src/ast/mod.rs b/src/ast/mod.rs index 886bea26d5..ae16f43848 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -67,10 +67,11 @@ pub use self::ddl::{ AlterPolicyOperation, AlterSchema, AlterSchemaOperation, AlterTable, AlterTableAlgorithm, AlterTableLock, AlterTableOperation, AlterTableType, AlterType, AlterTypeAddValue, AlterTypeAddValuePosition, AlterTypeOperation, AlterTypeRename, AlterTypeRenameValue, - ClusteredBy, ColumnDef, ColumnOption, ColumnOptionDef, ColumnOptions, ColumnPolicy, - ColumnPolicyProperty, ConstraintCharacteristics, CreateCollation, CreateCollationDefinition, - CreateConnector, CreateDomain, CreateExtension, CreateFunction, CreateIndex, CreateOperator, - CreateOperatorClass, CreateOperatorFamily, CreatePolicy, CreatePolicyCommand, CreatePolicyType, + AggregateModifyKind, ClusteredBy, ColumnDef, ColumnOption, ColumnOptionDef, ColumnOptions, + ColumnPolicy, ColumnPolicyProperty, ConstraintCharacteristics, CreateAggregate, + CreateAggregateOption, CreateCollation, CreateCollationDefinition, CreateConnector, + CreateDomain, CreateExtension, CreateFunction, CreateIndex, CreateOperator, CreateOperatorClass, + CreateOperatorFamily, CreatePolicy, CreatePolicyCommand, CreatePolicyType, CreateTable, CreateTrigger, CreateView, Deduplicate, DeferrableInitial, DistStyle, DropBehavior, DropExtension, DropFunction, DropOperator, DropOperatorClass, DropOperatorFamily, DropOperatorSignature, DropPolicy, DropTrigger, ForValues, FunctionReturnType, GeneratedAs, @@ -3762,6 +3763,11 @@ pub enum Statement { /// See [PostgreSQL](https://www.postgresql.org/docs/current/sql-createopclass.html) CreateOperatorClass(CreateOperatorClass), /// ```sql + /// CREATE AGGREGATE + /// ``` + /// See [PostgreSQL](https://www.postgresql.org/docs/current/sql-createaggregate.html) + CreateAggregate(CreateAggregate), + /// ```sql /// ALTER TABLE /// ``` AlterTable(AlterTable), @@ -5549,6 +5555,7 @@ impl fmt::Display for Statement { create_operator_family.fmt(f) } Statement::CreateOperatorClass(create_operator_class) => create_operator_class.fmt(f), + Statement::CreateAggregate(create_aggregate) => create_aggregate.fmt(f), Statement::AlterTable(alter_table) => write!(f, "{alter_table}"), Statement::AlterIndex { name, operation } => { write!(f, "ALTER INDEX {name} {operation}") From 9af8b16a061ddaf4096f14dec1fa9907b4761357 Mon Sep 17 00:00:00 2001 From: Filipe Guerreiro Date: Fri, 17 Apr 2026 11:24:13 +0900 Subject: [PATCH 02/17] feat(parser): parse CREATE AGGREGATE Wire AGGREGATE into the CREATE dispatch (before the or_replace error branch so CREATE OR REPLACE AGGREGATE is accepted). parse_create_aggregate parses the name, argument-type list, and the options block. Each recognised option keyword dispatches to parse_create_aggregate_option which produces the typed CreateAggregateOption variant. --- src/parser/mod.rs | 190 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 190 insertions(+) diff --git a/src/parser/mod.rs b/src/parser/mod.rs index 7501919a0c..f23fd3b056 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -5172,6 +5172,8 @@ impl<'a> Parser<'a> { self.parse_create_secret(or_replace, temporary, persistent) } else if self.parse_keyword(Keyword::USER) { self.parse_create_user(or_replace).map(Into::into) + } else if self.parse_keyword(Keyword::AGGREGATE) { + self.parse_create_aggregate(or_replace).map(Into::into) } else if or_replace { self.expected_ref( "[EXTERNAL] TABLE or [MATERIALIZED] VIEW or FUNCTION after CREATE OR REPLACE", @@ -7209,6 +7211,194 @@ impl<'a> Parser<'a> { }) } + /// Parse a [Statement::CreateAggregate] + /// + /// [PostgreSQL Documentation](https://www.postgresql.org/docs/current/sql-createaggregate.html) + pub fn parse_create_aggregate( + &mut self, + or_replace: bool, + ) -> Result { + let name = self.parse_object_name(false)?; + + // Argument type list: `(input_data_type [, ...])` or `(*)` for zero-arg. + self.expect_token(&Token::LParen)?; + let args = if self.consume_token(&Token::Mul) { + // zero-argument aggregate written as `(*)` — treat as empty arg list. + vec![] + } else if self.consume_token(&Token::RParen) { + self.prev_token(); + vec![] + } else { + let parsed = self.parse_comma_separated(|p| p.parse_data_type())?; + parsed + }; + self.expect_token(&Token::RParen)?; + + // Options block: `( SFUNC = ..., STYPE = ..., ... )` + self.expect_token(&Token::LParen)?; + let mut options: Vec = Vec::new(); + loop { + let token = self.next_token(); + match &token.token { + Token::RParen => break, + Token::Comma => continue, + Token::Word(word) => { + let option = self.parse_create_aggregate_option(&word.value.to_uppercase())?; + options.push(option); + } + other => { + return Err(ParserError::ParserError(format!( + "Unexpected token in CREATE AGGREGATE options: {other:?}" + ))); + } + } + } + + Ok(CreateAggregate { + or_replace, + name, + args, + options, + }) + } + + fn parse_create_aggregate_option( + &mut self, + key: &str, + ) -> Result { + match key { + "SFUNC" => { + self.expect_token(&Token::Eq)?; + Ok(CreateAggregateOption::Sfunc( + self.parse_object_name(false)?, + )) + } + "STYPE" => { + self.expect_token(&Token::Eq)?; + Ok(CreateAggregateOption::Stype(self.parse_data_type()?)) + } + "SSPACE" => { + self.expect_token(&Token::Eq)?; + let size = self.parse_literal_uint()?; + Ok(CreateAggregateOption::Sspace(size)) + } + "FINALFUNC" => { + self.expect_token(&Token::Eq)?; + Ok(CreateAggregateOption::Finalfunc( + self.parse_object_name(false)?, + )) + } + "FINALFUNC_EXTRA" => Ok(CreateAggregateOption::FinalfuncExtra), + "FINALFUNC_MODIFY" => { + self.expect_token(&Token::Eq)?; + Ok(CreateAggregateOption::FinalfuncModify( + self.parse_aggregate_modify_kind()?, + )) + } + "COMBINEFUNC" => { + self.expect_token(&Token::Eq)?; + Ok(CreateAggregateOption::Combinefunc( + self.parse_object_name(false)?, + )) + } + "SERIALFUNC" => { + self.expect_token(&Token::Eq)?; + Ok(CreateAggregateOption::Serialfunc( + self.parse_object_name(false)?, + )) + } + "DESERIALFUNC" => { + self.expect_token(&Token::Eq)?; + Ok(CreateAggregateOption::Deserialfunc( + self.parse_object_name(false)?, + )) + } + "INITCOND" => { + self.expect_token(&Token::Eq)?; + Ok(CreateAggregateOption::Initcond(self.parse_value()?.value)) + } + "MSFUNC" => { + self.expect_token(&Token::Eq)?; + Ok(CreateAggregateOption::Msfunc( + self.parse_object_name(false)?, + )) + } + "MINVFUNC" => { + self.expect_token(&Token::Eq)?; + Ok(CreateAggregateOption::Minvfunc( + self.parse_object_name(false)?, + )) + } + "MSTYPE" => { + self.expect_token(&Token::Eq)?; + Ok(CreateAggregateOption::Mstype(self.parse_data_type()?)) + } + "MSSPACE" => { + self.expect_token(&Token::Eq)?; + let size = self.parse_literal_uint()?; + Ok(CreateAggregateOption::Msspace(size)) + } + "MFINALFUNC" => { + self.expect_token(&Token::Eq)?; + Ok(CreateAggregateOption::Mfinalfunc( + self.parse_object_name(false)?, + )) + } + "MFINALFUNC_EXTRA" => Ok(CreateAggregateOption::MfinalfuncExtra), + "MFINALFUNC_MODIFY" => { + self.expect_token(&Token::Eq)?; + Ok(CreateAggregateOption::MfinalfuncModify( + self.parse_aggregate_modify_kind()?, + )) + } + "MINITCOND" => { + self.expect_token(&Token::Eq)?; + Ok(CreateAggregateOption::Minitcond(self.parse_value()?.value)) + } + "SORTOP" => { + self.expect_token(&Token::Eq)?; + Ok(CreateAggregateOption::Sortop( + self.parse_object_name(false)?, + )) + } + "PARALLEL" => { + self.expect_token(&Token::Eq)?; + let parallel = match self.expect_one_of_keywords(&[ + Keyword::SAFE, + Keyword::RESTRICTED, + Keyword::UNSAFE, + ])? { + Keyword::SAFE => FunctionParallel::Safe, + Keyword::RESTRICTED => FunctionParallel::Restricted, + Keyword::UNSAFE => FunctionParallel::Unsafe, + _ => unreachable!(), + }; + Ok(CreateAggregateOption::Parallel(parallel)) + } + "HYPOTHETICAL" => Ok(CreateAggregateOption::Hypothetical), + other => Err(ParserError::ParserError(format!( + "Unknown CREATE AGGREGATE option: {other}" + ))), + } + } + + fn parse_aggregate_modify_kind(&mut self) -> Result { + let token = self.next_token(); + match &token.token { + Token::Word(word) => match word.value.to_uppercase().as_str() { + "READ_ONLY" => Ok(AggregateModifyKind::ReadOnly), + "SHAREABLE" => Ok(AggregateModifyKind::Shareable), + "READ_WRITE" => Ok(AggregateModifyKind::ReadWrite), + other => Err(ParserError::ParserError(format!( + "Expected READ_ONLY, SHAREABLE, or READ_WRITE, got: {other}" + ))), + }, + other => Err(ParserError::ParserError(format!( + "Expected READ_ONLY, SHAREABLE, or READ_WRITE, got: {other:?}" + ))), + } + } + /// Parse a [Statement::CreateOperatorFamily] /// /// [PostgreSQL Documentation](https://www.postgresql.org/docs/current/sql-createopfamily.html) From bb533cf51d41466bef004eb815a1d921ba916617 Mon Sep 17 00:00:00 2001 From: Filipe Guerreiro Date: Fri, 17 Apr 2026 11:24:20 +0900 Subject: [PATCH 03/17] test: add CREATE AGGREGATE round-trip tests Three tests covering: basic old-style aggregate (SFUNC/STYPE/FINALFUNC/INITCOND), CREATE OR REPLACE with PARALLEL = SAFE, and moving-aggregate options (MSFUNC/MINVFUNC/MSTYPE/MFINALFUNC_EXTRA/MFINALFUNC_MODIFY). All use pg().verified_stmt() to assert parse-then-display round-trips identically. --- tests/sqlparser_postgres.rs | 60 +++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/tests/sqlparser_postgres.rs b/tests/sqlparser_postgres.rs index 86315b1ef9..ecdc2327c8 100644 --- a/tests/sqlparser_postgres.rs +++ b/tests/sqlparser_postgres.rs @@ -9221,3 +9221,63 @@ fn parse_lock_table() { } } } + +#[test] +fn parse_create_aggregate_basic() { + let sql = "CREATE AGGREGATE myavg (NUMERIC) (SFUNC = numeric_avg_accum, STYPE = internal, FINALFUNC = numeric_avg, INITCOND = '0')"; + let stmt = pg().verified_stmt(sql); + match stmt { + Statement::CreateAggregate(agg) => { + assert!(!agg.or_replace); + assert_eq!(agg.name.to_string(), "myavg"); + assert_eq!(agg.args.len(), 1); + assert_eq!(agg.args[0].to_string(), "NUMERIC"); + assert_eq!(agg.options.len(), 4); + assert_eq!( + agg.options[0].to_string(), + "SFUNC = numeric_avg_accum" + ); + assert_eq!(agg.options[1].to_string(), "STYPE = internal"); + assert_eq!(agg.options[2].to_string(), "FINALFUNC = numeric_avg"); + assert_eq!(agg.options[3].to_string(), "INITCOND = '0'"); + } + _ => panic!("Expected CreateAggregate, got: {stmt:?}"), + } +} + +#[test] +fn parse_create_aggregate_or_replace_with_parallel() { + let sql = "CREATE OR REPLACE AGGREGATE sum2 (INT4, INT4) (SFUNC = int4pl, STYPE = INT4, PARALLEL = SAFE)"; + let stmt = pg().verified_stmt(sql); + match stmt { + Statement::CreateAggregate(agg) => { + assert!(agg.or_replace); + assert_eq!(agg.name.to_string(), "sum2"); + assert_eq!(agg.args.len(), 2); + assert_eq!(agg.options.len(), 3); + assert_eq!(agg.options[2].to_string(), "PARALLEL = SAFE"); + } + _ => panic!("Expected CreateAggregate, got: {stmt:?}"), + } +} + +#[test] +fn parse_create_aggregate_with_moving_aggregate_options() { + let sql = "CREATE AGGREGATE moving_sum (FLOAT8) (SFUNC = float8pl, STYPE = FLOAT8, MSFUNC = float8pl, MINVFUNC = float8mi, MSTYPE = FLOAT8, MFINALFUNC_EXTRA, MFINALFUNC_MODIFY = READ_ONLY)"; + let stmt = pg().verified_stmt(sql); + match stmt { + Statement::CreateAggregate(agg) => { + assert!(!agg.or_replace); + assert_eq!(agg.name.to_string(), "moving_sum"); + assert_eq!(agg.args.len(), 1); + assert_eq!(agg.options.len(), 7); + assert_eq!(agg.options[4].to_string(), "MSTYPE = FLOAT8"); + assert_eq!(agg.options[5].to_string(), "MFINALFUNC_EXTRA"); + assert_eq!( + agg.options[6].to_string(), + "MFINALFUNC_MODIFY = READ_ONLY" + ); + } + _ => panic!("Expected CreateAggregate, got: {stmt:?}"), + } +} From a0997d4f9dec77243239521f2532a901ce4a2a23 Mon Sep 17 00:00:00 2001 From: Filipe Guerreiro Date: Sat, 18 Apr 2026 10:00:28 +0900 Subject: [PATCH 04/17] fix(spans): add Span::empty() arm for CreateAggregate PR #7 added the Statement::CreateAggregate variant but omitted the corresponding match arm in the Spanned impl for Statement. Fork CI never ran on the PR so this was not caught before merge. --- src/ast/spans.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ast/spans.rs b/src/ast/spans.rs index adc1443fc7..2253e7560d 100644 --- a/src/ast/spans.rs +++ b/src/ast/spans.rs @@ -518,6 +518,7 @@ impl Spanned for Statement { Statement::Vacuum(..) => Span::empty(), Statement::AlterUser(..) => Span::empty(), Statement::Reset(..) => Span::empty(), + Statement::CreateAggregate(_) => Span::empty(), } } } From acdbda2354a8e71cbd9cc1f6c744af9c5e2871d7 Mon Sep 17 00:00:00 2001 From: Filipe Guerreiro Date: Sat, 18 Apr 2026 18:17:58 +0900 Subject: [PATCH 05/17] chore: apply cargo fmt and sort new keywords --- src/ast/mod.rs | 20 ++++++++++---------- src/parser/mod.rs | 4 +--- tests/sqlparser_postgres.rs | 10 ++-------- 3 files changed, 13 insertions(+), 21 deletions(-) diff --git a/src/ast/mod.rs b/src/ast/mod.rs index ae16f43848..ccd9cab21f 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -60,18 +60,18 @@ pub use self::dcl::{ SetConfigValue, Use, }; pub use self::ddl::{ - Alignment, AlterCollation, AlterCollationOperation, AlterColumnOperation, AlterConnectorOwner, - AlterFunction, AlterFunctionAction, AlterFunctionKind, AlterFunctionOperation, - AlterIndexOperation, AlterOperator, AlterOperatorClass, AlterOperatorClassOperation, - AlterOperatorFamily, AlterOperatorFamilyOperation, AlterOperatorOperation, AlterPolicy, - AlterPolicyOperation, AlterSchema, AlterSchemaOperation, AlterTable, AlterTableAlgorithm, - AlterTableLock, AlterTableOperation, AlterTableType, AlterType, AlterTypeAddValue, - AlterTypeAddValuePosition, AlterTypeOperation, AlterTypeRename, AlterTypeRenameValue, - AggregateModifyKind, ClusteredBy, ColumnDef, ColumnOption, ColumnOptionDef, ColumnOptions, + AggregateModifyKind, Alignment, AlterCollation, AlterCollationOperation, AlterColumnOperation, + AlterConnectorOwner, AlterFunction, AlterFunctionAction, AlterFunctionKind, + AlterFunctionOperation, AlterIndexOperation, AlterOperator, AlterOperatorClass, + AlterOperatorClassOperation, AlterOperatorFamily, AlterOperatorFamilyOperation, + AlterOperatorOperation, AlterPolicy, AlterPolicyOperation, AlterSchema, AlterSchemaOperation, + AlterTable, AlterTableAlgorithm, AlterTableLock, AlterTableOperation, AlterTableType, + AlterType, AlterTypeAddValue, AlterTypeAddValuePosition, AlterTypeOperation, AlterTypeRename, + AlterTypeRenameValue, ClusteredBy, ColumnDef, ColumnOption, ColumnOptionDef, ColumnOptions, ColumnPolicy, ColumnPolicyProperty, ConstraintCharacteristics, CreateAggregate, CreateAggregateOption, CreateCollation, CreateCollationDefinition, CreateConnector, - CreateDomain, CreateExtension, CreateFunction, CreateIndex, CreateOperator, CreateOperatorClass, - CreateOperatorFamily, CreatePolicy, CreatePolicyCommand, CreatePolicyType, + CreateDomain, CreateExtension, CreateFunction, CreateIndex, CreateOperator, + CreateOperatorClass, CreateOperatorFamily, CreatePolicy, CreatePolicyCommand, CreatePolicyType, CreateTable, CreateTrigger, CreateView, Deduplicate, DeferrableInitial, DistStyle, DropBehavior, DropExtension, DropFunction, DropOperator, DropOperatorClass, DropOperatorFamily, DropOperatorSignature, DropPolicy, DropTrigger, ForValues, FunctionReturnType, GeneratedAs, diff --git a/src/parser/mod.rs b/src/parser/mod.rs index f23fd3b056..82c57f4fda 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -7269,9 +7269,7 @@ impl<'a> Parser<'a> { match key { "SFUNC" => { self.expect_token(&Token::Eq)?; - Ok(CreateAggregateOption::Sfunc( - self.parse_object_name(false)?, - )) + Ok(CreateAggregateOption::Sfunc(self.parse_object_name(false)?)) } "STYPE" => { self.expect_token(&Token::Eq)?; diff --git a/tests/sqlparser_postgres.rs b/tests/sqlparser_postgres.rs index ecdc2327c8..0382f2fc4b 100644 --- a/tests/sqlparser_postgres.rs +++ b/tests/sqlparser_postgres.rs @@ -9233,10 +9233,7 @@ fn parse_create_aggregate_basic() { assert_eq!(agg.args.len(), 1); assert_eq!(agg.args[0].to_string(), "NUMERIC"); assert_eq!(agg.options.len(), 4); - assert_eq!( - agg.options[0].to_string(), - "SFUNC = numeric_avg_accum" - ); + assert_eq!(agg.options[0].to_string(), "SFUNC = numeric_avg_accum"); assert_eq!(agg.options[1].to_string(), "STYPE = internal"); assert_eq!(agg.options[2].to_string(), "FINALFUNC = numeric_avg"); assert_eq!(agg.options[3].to_string(), "INITCOND = '0'"); @@ -9273,10 +9270,7 @@ fn parse_create_aggregate_with_moving_aggregate_options() { assert_eq!(agg.options.len(), 7); assert_eq!(agg.options[4].to_string(), "MSTYPE = FLOAT8"); assert_eq!(agg.options[5].to_string(), "MFINALFUNC_EXTRA"); - assert_eq!( - agg.options[6].to_string(), - "MFINALFUNC_MODIFY = READ_ONLY" - ); + assert_eq!(agg.options[6].to_string(), "MFINALFUNC_MODIFY = READ_ONLY"); } _ => panic!("Expected CreateAggregate, got: {stmt:?}"), } From 31adcd60934d0c64b6158c98ed3d8bf9e62f8bb8 Mon Sep 17 00:00:00 2001 From: Filipe Guerreiro Date: Sat, 18 Apr 2026 18:21:10 +0900 Subject: [PATCH 06/17] fix(clippy): replace let-then-return and unreachable!() with explicit error --- src/parser/mod.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/parser/mod.rs b/src/parser/mod.rs index 82c57f4fda..8555c6812c 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -7229,8 +7229,7 @@ impl<'a> Parser<'a> { self.prev_token(); vec![] } else { - let parsed = self.parse_comma_separated(|p| p.parse_data_type())?; - parsed + self.parse_comma_separated(|p| p.parse_data_type())? }; self.expect_token(&Token::RParen)?; @@ -7369,7 +7368,11 @@ impl<'a> Parser<'a> { Keyword::SAFE => FunctionParallel::Safe, Keyword::RESTRICTED => FunctionParallel::Restricted, Keyword::UNSAFE => FunctionParallel::Unsafe, - _ => unreachable!(), + unexpected_keyword => { + return Err(ParserError::ParserError(format!( + "Internal parser error: unexpected keyword `{unexpected_keyword}` in PARALLEL" + ))) + } }; Ok(CreateAggregateOption::Parallel(parallel)) } From 164e92c255252ced5eeaa07eb7f35a730aab5063 Mon Sep 17 00:00:00 2001 From: Filipe Guerreiro Date: Sat, 18 Apr 2026 19:07:54 +0900 Subject: [PATCH 07/17] =?UTF-8?q?review:=20address=20feedback=20=E2=80=94?= =?UTF-8?q?=20operator=20name,=20value=20spans,=20parser=20idioms?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - SORTOP now parses via parse_operator_name so bare operators (SORTOP = <) work correctly. - INITCOND / MINITCOND now store ValueWithSpan, preserving source location and matching the rest of the DDL layer. - Replace the hand-rolled option loop with parse_comma_separated, rejecting leading and doubled commas. - Simplify empty arg-list detection (no more prev_token dance). - Replace the PARALLEL match-with-fallthrough with the if/else-if shape used elsewhere in the parser. - Extend the 'after CREATE OR REPLACE' error message to mention AGGREGATE. --- src/ast/ddl.rs | 4 +-- src/parser/mod.rs | 66 +++++++++++++++++------------------------------ 2 files changed, 26 insertions(+), 44 deletions(-) diff --git a/src/ast/ddl.rs b/src/ast/ddl.rs index 4f511dfb95..206f0f442f 100644 --- a/src/ast/ddl.rs +++ b/src/ast/ddl.rs @@ -5826,7 +5826,7 @@ pub enum CreateAggregateOption { /// `DESERIALFUNC = deserial_function` Deserialfunc(ObjectName), /// `INITCOND = initial_condition` (a string literal) - Initcond(Value), + Initcond(ValueWithSpan), /// `MSFUNC = moving_state_transition_function` Msfunc(ObjectName), /// `MINVFUNC = moving_inverse_transition_function` @@ -5842,7 +5842,7 @@ pub enum CreateAggregateOption { /// `MFINALFUNC_MODIFY = { READ_ONLY | SHAREABLE | READ_WRITE }` MfinalfuncModify(AggregateModifyKind), /// `MINITCOND = moving_initial_condition` (a string literal) - Minitcond(Value), + Minitcond(ValueWithSpan), /// `SORTOP = sort_operator` Sortop(ObjectName), /// `PARALLEL = { SAFE | RESTRICTED | UNSAFE }` diff --git a/src/parser/mod.rs b/src/parser/mod.rs index 8555c6812c..82dcbfc054 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -5176,7 +5176,7 @@ impl<'a> Parser<'a> { self.parse_create_aggregate(or_replace).map(Into::into) } else if or_replace { self.expected_ref( - "[EXTERNAL] TABLE or [MATERIALIZED] VIEW or FUNCTION after CREATE OR REPLACE", + "[EXTERNAL] TABLE or [MATERIALIZED] VIEW or FUNCTION or AGGREGATE after CREATE OR REPLACE", self.peek_token_ref(), ) } else if self.parse_keyword(Keyword::EXTENSION) { @@ -7222,36 +7222,22 @@ impl<'a> Parser<'a> { // Argument type list: `(input_data_type [, ...])` or `(*)` for zero-arg. self.expect_token(&Token::LParen)?; - let args = if self.consume_token(&Token::Mul) { - // zero-argument aggregate written as `(*)` — treat as empty arg list. - vec![] - } else if self.consume_token(&Token::RParen) { - self.prev_token(); + let args = if self.consume_token(&Token::Mul) + || self.peek_token().token == Token::RParen + { vec![] } else { self.parse_comma_separated(|p| p.parse_data_type())? }; self.expect_token(&Token::RParen)?; - // Options block: `( SFUNC = ..., STYPE = ..., ... )` + // Options block: `( SFUNC = ..., STYPE = ..., ... )`. self.expect_token(&Token::LParen)?; - let mut options: Vec = Vec::new(); - loop { - let token = self.next_token(); - match &token.token { - Token::RParen => break, - Token::Comma => continue, - Token::Word(word) => { - let option = self.parse_create_aggregate_option(&word.value.to_uppercase())?; - options.push(option); - } - other => { - return Err(ParserError::ParserError(format!( - "Unexpected token in CREATE AGGREGATE options: {other:?}" - ))); - } - } - } + let options = self.parse_comma_separated(|parser| { + let key = parser.parse_identifier()?; + parser.parse_create_aggregate_option(&key.value.to_uppercase()) + })?; + self.expect_token(&Token::RParen)?; Ok(CreateAggregate { or_replace, @@ -7312,7 +7298,7 @@ impl<'a> Parser<'a> { } "INITCOND" => { self.expect_token(&Token::Eq)?; - Ok(CreateAggregateOption::Initcond(self.parse_value()?.value)) + Ok(CreateAggregateOption::Initcond(self.parse_value()?)) } "MSFUNC" => { self.expect_token(&Token::Eq)?; @@ -7350,29 +7336,25 @@ impl<'a> Parser<'a> { } "MINITCOND" => { self.expect_token(&Token::Eq)?; - Ok(CreateAggregateOption::Minitcond(self.parse_value()?.value)) + Ok(CreateAggregateOption::Minitcond(self.parse_value()?)) } "SORTOP" => { self.expect_token(&Token::Eq)?; - Ok(CreateAggregateOption::Sortop( - self.parse_object_name(false)?, - )) + Ok(CreateAggregateOption::Sortop(self.parse_operator_name()?)) } "PARALLEL" => { self.expect_token(&Token::Eq)?; - let parallel = match self.expect_one_of_keywords(&[ - Keyword::SAFE, - Keyword::RESTRICTED, - Keyword::UNSAFE, - ])? { - Keyword::SAFE => FunctionParallel::Safe, - Keyword::RESTRICTED => FunctionParallel::Restricted, - Keyword::UNSAFE => FunctionParallel::Unsafe, - unexpected_keyword => { - return Err(ParserError::ParserError(format!( - "Internal parser error: unexpected keyword `{unexpected_keyword}` in PARALLEL" - ))) - } + let parallel = if self.parse_keyword(Keyword::SAFE) { + FunctionParallel::Safe + } else if self.parse_keyword(Keyword::RESTRICTED) { + FunctionParallel::Restricted + } else if self.parse_keyword(Keyword::UNSAFE) { + FunctionParallel::Unsafe + } else { + return self.expected_ref( + "SAFE, RESTRICTED, or UNSAFE after PARALLEL =", + self.peek_token_ref(), + ); }; Ok(CreateAggregateOption::Parallel(parallel)) } From fc3e279a7e8db85d130ac21d00f946a222b4a2c7 Mon Sep 17 00:00:00 2001 From: Filipe Guerreiro Date: Sat, 18 Apr 2026 20:21:57 +0900 Subject: [PATCH 08/17] review: implement real Spanned for CreateAggregate Return the name's span instead of Span::empty() to match the sibling Create* arms. --- src/ast/spans.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ast/spans.rs b/src/ast/spans.rs index 2253e7560d..34ec5d2332 100644 --- a/src/ast/spans.rs +++ b/src/ast/spans.rs @@ -518,7 +518,7 @@ impl Spanned for Statement { Statement::Vacuum(..) => Span::empty(), Statement::AlterUser(..) => Span::empty(), Statement::Reset(..) => Span::empty(), - Statement::CreateAggregate(_) => Span::empty(), + Statement::CreateAggregate(stmt) => stmt.name.span(), } } } From 8df50b0620c088ba8929c164cff83ace5ac7849f Mon Sep 17 00:00:00 2001 From: Filipe Guerreiro Date: Sat, 18 Apr 2026 20:24:11 +0900 Subject: [PATCH 09/17] chore: cargo fmt --- src/parser/mod.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/parser/mod.rs b/src/parser/mod.rs index 82dcbfc054..c1224a126a 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -7222,9 +7222,7 @@ impl<'a> Parser<'a> { // Argument type list: `(input_data_type [, ...])` or `(*)` for zero-arg. self.expect_token(&Token::LParen)?; - let args = if self.consume_token(&Token::Mul) - || self.peek_token().token == Token::RParen - { + let args = if self.consume_token(&Token::Mul) || self.peek_token().token == Token::RParen { vec![] } else { self.parse_comma_separated(|p| p.parse_data_type())? From 7671da04db6aacc9e6a54a23baa076f8f44dc806 Mon Sep 17 00:00:00 2001 From: Filipe Guerreiro Date: Mon, 18 May 2026 18:12:43 +0900 Subject: [PATCH 10/17] Address review feedback for CREATE AGGREGATE - Rename abbreviated CreateAggregateOption variants to full words - Preserve (*) wildcard arg list via CreateAggregate::star_args - Implement Spanned for CreateAggregate and delegate from Statement - Parse aggregate args as OperateFunctionArg so VARIADIC/named args round-trip instead of being dropped - Add verified_stmt coverage for HYPOTHETICAL, SORTOP, schema-qualified SORTOP, FINALFUNC_MODIFY, SSPACE, COMBINEFUNC, SERIALFUNC/DESERIALFUNC, star args, and named/variadic args - Switch CREATE AGGREGATE tests to pg_and_generic(), use display_comma_separated for options, extract parse_function_parallel --- src/ast/ddl.rs | 96 ++++++++++++++++++------------------- src/ast/spans.rs | 12 +++-- src/parser/mod.rs | 85 ++++++++++++++++++-------------- tests/sqlparser_postgres.rs | 57 ++++++++++++++++++++-- 4 files changed, 161 insertions(+), 89 deletions(-) diff --git a/src/ast/ddl.rs b/src/ast/ddl.rs index 227f0e90fb..09610dde5a 100644 --- a/src/ast/ddl.rs +++ b/src/ast/ddl.rs @@ -5833,8 +5833,11 @@ pub struct CreateAggregate { pub or_replace: bool, /// The aggregate name (can be schema-qualified). pub name: ObjectName, - /// Input argument types. Empty for zero-argument aggregates. - pub args: Vec, + /// Input arguments. Empty for zero-argument aggregates or `(*)`. + pub args: Vec, + /// True if the argument list was the wildcard form `(*)` (used by + /// zero-argument aggregates such as `count(*)`). + pub star_args: bool, /// The options listed inside the required parentheses after the argument /// list (e.g. `SFUNC`, `STYPE`, `FINALFUNC`, `PARALLEL`, …). pub options: Vec, @@ -5847,15 +5850,12 @@ impl fmt::Display for CreateAggregate { write!(f, " OR REPLACE")?; } write!(f, " AGGREGATE {}", self.name)?; - write!(f, " ({})", display_comma_separated(&self.args))?; - write!(f, " (")?; - for (i, option) in self.options.iter().enumerate() { - if i > 0 { - write!(f, ", ")?; - } - write!(f, "{option}")?; + if self.star_args && self.args.is_empty() { + write!(f, " (*)")?; + } else { + write!(f, " ({})", display_comma_separated(&self.args))?; } - write!(f, ")") + write!(f, " ({})", display_comma_separated(&self.options)) } } @@ -5873,43 +5873,43 @@ impl From for crate::ast::Statement { #[cfg_attr(feature = "visitor", derive(Visit, VisitMut))] pub enum CreateAggregateOption { /// `SFUNC = state_transition_function` - Sfunc(ObjectName), + StateTransitionFunction(ObjectName), /// `STYPE = state_data_type` - Stype(DataType), + StateDataType(DataType), /// `SSPACE = state_data_size` (in bytes) - Sspace(u64), + StateDataSize(u64), /// `FINALFUNC = final_function` - Finalfunc(ObjectName), + FinalFunction(ObjectName), /// `FINALFUNC_EXTRA` — pass extra dummy arguments to the final function. - FinalfuncExtra, + FinalFunctionExtra, /// `FINALFUNC_MODIFY = { READ_ONLY | SHAREABLE | READ_WRITE }` - FinalfuncModify(AggregateModifyKind), + FinalFunctionModify(AggregateModifyKind), /// `COMBINEFUNC = combine_function` - Combinefunc(ObjectName), + CombineFunction(ObjectName), /// `SERIALFUNC = serial_function` - Serialfunc(ObjectName), + SerialFunction(ObjectName), /// `DESERIALFUNC = deserial_function` - Deserialfunc(ObjectName), + DeserialFunction(ObjectName), /// `INITCOND = initial_condition` (a string literal) - Initcond(ValueWithSpan), + InitialCondition(ValueWithSpan), /// `MSFUNC = moving_state_transition_function` - Msfunc(ObjectName), + MovingStateTransitionFunction(ObjectName), /// `MINVFUNC = moving_inverse_transition_function` - Minvfunc(ObjectName), + MovingInverseTransitionFunction(ObjectName), /// `MSTYPE = moving_state_data_type` - Mstype(DataType), + MovingStateDataType(DataType), /// `MSSPACE = moving_state_data_size` (in bytes) - Msspace(u64), + MovingStateDataSize(u64), /// `MFINALFUNC = moving_final_function` - Mfinalfunc(ObjectName), + MovingFinalFunction(ObjectName), /// `MFINALFUNC_EXTRA` - MfinalfuncExtra, + MovingFinalFunctionExtra, /// `MFINALFUNC_MODIFY = { READ_ONLY | SHAREABLE | READ_WRITE }` - MfinalfuncModify(AggregateModifyKind), + MovingFinalFunctionModify(AggregateModifyKind), /// `MINITCOND = moving_initial_condition` (a string literal) - Minitcond(ValueWithSpan), + MovingInitialCondition(ValueWithSpan), /// `SORTOP = sort_operator` - Sortop(ObjectName), + SortOperator(ObjectName), /// `PARALLEL = { SAFE | RESTRICTED | UNSAFE }` Parallel(FunctionParallel), /// `HYPOTHETICAL` — marks the aggregate as hypothetical-set. @@ -5919,25 +5919,25 @@ pub enum CreateAggregateOption { impl fmt::Display for CreateAggregateOption { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self { - Self::Sfunc(name) => write!(f, "SFUNC = {name}"), - Self::Stype(data_type) => write!(f, "STYPE = {data_type}"), - Self::Sspace(size) => write!(f, "SSPACE = {size}"), - Self::Finalfunc(name) => write!(f, "FINALFUNC = {name}"), - Self::FinalfuncExtra => write!(f, "FINALFUNC_EXTRA"), - Self::FinalfuncModify(kind) => write!(f, "FINALFUNC_MODIFY = {kind}"), - Self::Combinefunc(name) => write!(f, "COMBINEFUNC = {name}"), - Self::Serialfunc(name) => write!(f, "SERIALFUNC = {name}"), - Self::Deserialfunc(name) => write!(f, "DESERIALFUNC = {name}"), - Self::Initcond(cond) => write!(f, "INITCOND = {cond}"), - Self::Msfunc(name) => write!(f, "MSFUNC = {name}"), - Self::Minvfunc(name) => write!(f, "MINVFUNC = {name}"), - Self::Mstype(data_type) => write!(f, "MSTYPE = {data_type}"), - Self::Msspace(size) => write!(f, "MSSPACE = {size}"), - Self::Mfinalfunc(name) => write!(f, "MFINALFUNC = {name}"), - Self::MfinalfuncExtra => write!(f, "MFINALFUNC_EXTRA"), - Self::MfinalfuncModify(kind) => write!(f, "MFINALFUNC_MODIFY = {kind}"), - Self::Minitcond(cond) => write!(f, "MINITCOND = {cond}"), - Self::Sortop(name) => write!(f, "SORTOP = {name}"), + Self::StateTransitionFunction(name) => write!(f, "SFUNC = {name}"), + Self::StateDataType(data_type) => write!(f, "STYPE = {data_type}"), + Self::StateDataSize(size) => write!(f, "SSPACE = {size}"), + Self::FinalFunction(name) => write!(f, "FINALFUNC = {name}"), + Self::FinalFunctionExtra => write!(f, "FINALFUNC_EXTRA"), + Self::FinalFunctionModify(kind) => write!(f, "FINALFUNC_MODIFY = {kind}"), + Self::CombineFunction(name) => write!(f, "COMBINEFUNC = {name}"), + Self::SerialFunction(name) => write!(f, "SERIALFUNC = {name}"), + Self::DeserialFunction(name) => write!(f, "DESERIALFUNC = {name}"), + Self::InitialCondition(cond) => write!(f, "INITCOND = {cond}"), + Self::MovingStateTransitionFunction(name) => write!(f, "MSFUNC = {name}"), + Self::MovingInverseTransitionFunction(name) => write!(f, "MINVFUNC = {name}"), + Self::MovingStateDataType(data_type) => write!(f, "MSTYPE = {data_type}"), + Self::MovingStateDataSize(size) => write!(f, "MSSPACE = {size}"), + Self::MovingFinalFunction(name) => write!(f, "MFINALFUNC = {name}"), + Self::MovingFinalFunctionExtra => write!(f, "MFINALFUNC_EXTRA"), + Self::MovingFinalFunctionModify(kind) => write!(f, "MFINALFUNC_MODIFY = {kind}"), + Self::MovingInitialCondition(cond) => write!(f, "MINITCOND = {cond}"), + Self::SortOperator(name) => write!(f, "SORTOP = {name}"), Self::Parallel(parallel) => { let kind = match parallel { FunctionParallel::Safe => "SAFE", diff --git a/src/ast/spans.rs b/src/ast/spans.rs index 224208f520..d9eff97930 100644 --- a/src/ast/spans.rs +++ b/src/ast/spans.rs @@ -18,8 +18,8 @@ use crate::{ ast::{ ddl::AlterSchema, query::SelectItemQualifiedWildcardKind, AlterSchemaOperation, AlterTable, - ColumnOptions, CreateOperator, CreateOperatorClass, CreateOperatorFamily, CreateView, - ExportData, Owner, TypedString, + ColumnOptions, CreateAggregate, CreateOperator, CreateOperatorClass, CreateOperatorFamily, + CreateView, ExportData, Owner, TypedString, }, tokenizer::TokenWithSpan, }; @@ -518,7 +518,7 @@ impl Spanned for Statement { Statement::Vacuum(..) => Span::empty(), Statement::AlterUser(..) => Span::empty(), Statement::Reset(..) => Span::empty(), - Statement::CreateAggregate(stmt) => stmt.name.span(), + Statement::CreateAggregate(create_aggregate) => create_aggregate.span(), } } } @@ -2498,6 +2498,12 @@ impl Spanned for AlterTable { } } +impl Spanned for CreateAggregate { + fn span(&self) -> Span { + Span::empty() + } +} + impl Spanned for CreateOperator { fn span(&self) -> Span { Span::empty() diff --git a/src/parser/mod.rs b/src/parser/mod.rs index 52e3d65ab5..cdb3006c6c 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -7235,12 +7235,13 @@ impl<'a> Parser<'a> { ) -> Result { let name = self.parse_object_name(false)?; - // Argument type list: `(input_data_type [, ...])` or `(*)` for zero-arg. + // Argument list: `(input_arg [, ...])` or `(*)` for zero-arg. self.expect_token(&Token::LParen)?; - let args = if self.consume_token(&Token::Mul) || self.peek_token().token == Token::RParen { + let star_args = self.consume_token(&Token::Mul); + let args = if star_args || self.peek_token().token == Token::RParen { vec![] } else { - self.parse_comma_separated(|p| p.parse_data_type())? + self.parse_comma_separated(Parser::parse_function_arg)? }; self.expect_token(&Token::RParen)?; @@ -7256,10 +7257,24 @@ impl<'a> Parser<'a> { or_replace, name, args, + star_args, options, }) } + /// Parse `PARALLEL` qualifier value: `{ SAFE | RESTRICTED | UNSAFE }`. + fn parse_function_parallel(&mut self) -> Result { + if self.parse_keyword(Keyword::SAFE) { + Ok(FunctionParallel::Safe) + } else if self.parse_keyword(Keyword::RESTRICTED) { + Ok(FunctionParallel::Restricted) + } else if self.parse_keyword(Keyword::UNSAFE) { + Ok(FunctionParallel::Unsafe) + } else { + self.expected_ref("one of SAFE | RESTRICTED | UNSAFE", self.peek_token_ref()) + } + } + fn parse_create_aggregate_option( &mut self, key: &str, @@ -7267,109 +7282,109 @@ impl<'a> Parser<'a> { match key { "SFUNC" => { self.expect_token(&Token::Eq)?; - Ok(CreateAggregateOption::Sfunc(self.parse_object_name(false)?)) + Ok(CreateAggregateOption::StateTransitionFunction( + self.parse_object_name(false)?, + )) } "STYPE" => { self.expect_token(&Token::Eq)?; - Ok(CreateAggregateOption::Stype(self.parse_data_type()?)) + Ok(CreateAggregateOption::StateDataType( + self.parse_data_type()?, + )) } "SSPACE" => { self.expect_token(&Token::Eq)?; let size = self.parse_literal_uint()?; - Ok(CreateAggregateOption::Sspace(size)) + Ok(CreateAggregateOption::StateDataSize(size)) } "FINALFUNC" => { self.expect_token(&Token::Eq)?; - Ok(CreateAggregateOption::Finalfunc( + Ok(CreateAggregateOption::FinalFunction( self.parse_object_name(false)?, )) } - "FINALFUNC_EXTRA" => Ok(CreateAggregateOption::FinalfuncExtra), + "FINALFUNC_EXTRA" => Ok(CreateAggregateOption::FinalFunctionExtra), "FINALFUNC_MODIFY" => { self.expect_token(&Token::Eq)?; - Ok(CreateAggregateOption::FinalfuncModify( + Ok(CreateAggregateOption::FinalFunctionModify( self.parse_aggregate_modify_kind()?, )) } "COMBINEFUNC" => { self.expect_token(&Token::Eq)?; - Ok(CreateAggregateOption::Combinefunc( + Ok(CreateAggregateOption::CombineFunction( self.parse_object_name(false)?, )) } "SERIALFUNC" => { self.expect_token(&Token::Eq)?; - Ok(CreateAggregateOption::Serialfunc( + Ok(CreateAggregateOption::SerialFunction( self.parse_object_name(false)?, )) } "DESERIALFUNC" => { self.expect_token(&Token::Eq)?; - Ok(CreateAggregateOption::Deserialfunc( + Ok(CreateAggregateOption::DeserialFunction( self.parse_object_name(false)?, )) } "INITCOND" => { self.expect_token(&Token::Eq)?; - Ok(CreateAggregateOption::Initcond(self.parse_value()?)) + Ok(CreateAggregateOption::InitialCondition(self.parse_value()?)) } "MSFUNC" => { self.expect_token(&Token::Eq)?; - Ok(CreateAggregateOption::Msfunc( + Ok(CreateAggregateOption::MovingStateTransitionFunction( self.parse_object_name(false)?, )) } "MINVFUNC" => { self.expect_token(&Token::Eq)?; - Ok(CreateAggregateOption::Minvfunc( + Ok(CreateAggregateOption::MovingInverseTransitionFunction( self.parse_object_name(false)?, )) } "MSTYPE" => { self.expect_token(&Token::Eq)?; - Ok(CreateAggregateOption::Mstype(self.parse_data_type()?)) + Ok(CreateAggregateOption::MovingStateDataType( + self.parse_data_type()?, + )) } "MSSPACE" => { self.expect_token(&Token::Eq)?; let size = self.parse_literal_uint()?; - Ok(CreateAggregateOption::Msspace(size)) + Ok(CreateAggregateOption::MovingStateDataSize(size)) } "MFINALFUNC" => { self.expect_token(&Token::Eq)?; - Ok(CreateAggregateOption::Mfinalfunc( + Ok(CreateAggregateOption::MovingFinalFunction( self.parse_object_name(false)?, )) } - "MFINALFUNC_EXTRA" => Ok(CreateAggregateOption::MfinalfuncExtra), + "MFINALFUNC_EXTRA" => Ok(CreateAggregateOption::MovingFinalFunctionExtra), "MFINALFUNC_MODIFY" => { self.expect_token(&Token::Eq)?; - Ok(CreateAggregateOption::MfinalfuncModify( + Ok(CreateAggregateOption::MovingFinalFunctionModify( self.parse_aggregate_modify_kind()?, )) } "MINITCOND" => { self.expect_token(&Token::Eq)?; - Ok(CreateAggregateOption::Minitcond(self.parse_value()?)) + Ok(CreateAggregateOption::MovingInitialCondition( + self.parse_value()?, + )) } "SORTOP" => { self.expect_token(&Token::Eq)?; - Ok(CreateAggregateOption::Sortop(self.parse_operator_name()?)) + Ok(CreateAggregateOption::SortOperator( + self.parse_operator_name()?, + )) } "PARALLEL" => { self.expect_token(&Token::Eq)?; - let parallel = if self.parse_keyword(Keyword::SAFE) { - FunctionParallel::Safe - } else if self.parse_keyword(Keyword::RESTRICTED) { - FunctionParallel::Restricted - } else if self.parse_keyword(Keyword::UNSAFE) { - FunctionParallel::Unsafe - } else { - return self.expected_ref( - "SAFE, RESTRICTED, or UNSAFE after PARALLEL =", - self.peek_token_ref(), - ); - }; - Ok(CreateAggregateOption::Parallel(parallel)) + Ok(CreateAggregateOption::Parallel( + self.parse_function_parallel()?, + )) } "HYPOTHETICAL" => Ok(CreateAggregateOption::Hypothetical), other => Err(ParserError::ParserError(format!( diff --git a/tests/sqlparser_postgres.rs b/tests/sqlparser_postgres.rs index 95a59fdd9d..8c7898db9d 100644 --- a/tests/sqlparser_postgres.rs +++ b/tests/sqlparser_postgres.rs @@ -9247,10 +9247,11 @@ fn parse_lock_table() { #[test] fn parse_create_aggregate_basic() { let sql = "CREATE AGGREGATE myavg (NUMERIC) (SFUNC = numeric_avg_accum, STYPE = internal, FINALFUNC = numeric_avg, INITCOND = '0')"; - let stmt = pg().verified_stmt(sql); + let stmt = pg_and_generic().verified_stmt(sql); match stmt { Statement::CreateAggregate(agg) => { assert!(!agg.or_replace); + assert!(!agg.star_args); assert_eq!(agg.name.to_string(), "myavg"); assert_eq!(agg.args.len(), 1); assert_eq!(agg.args[0].to_string(), "NUMERIC"); @@ -9267,7 +9268,7 @@ fn parse_create_aggregate_basic() { #[test] fn parse_create_aggregate_or_replace_with_parallel() { let sql = "CREATE OR REPLACE AGGREGATE sum2 (INT4, INT4) (SFUNC = int4pl, STYPE = INT4, PARALLEL = SAFE)"; - let stmt = pg().verified_stmt(sql); + let stmt = pg_and_generic().verified_stmt(sql); match stmt { Statement::CreateAggregate(agg) => { assert!(agg.or_replace); @@ -9283,7 +9284,7 @@ fn parse_create_aggregate_or_replace_with_parallel() { #[test] fn parse_create_aggregate_with_moving_aggregate_options() { let sql = "CREATE AGGREGATE moving_sum (FLOAT8) (SFUNC = float8pl, STYPE = FLOAT8, MSFUNC = float8pl, MINVFUNC = float8mi, MSTYPE = FLOAT8, MFINALFUNC_EXTRA, MFINALFUNC_MODIFY = READ_ONLY)"; - let stmt = pg().verified_stmt(sql); + let stmt = pg_and_generic().verified_stmt(sql); match stmt { Statement::CreateAggregate(agg) => { assert!(!agg.or_replace); @@ -9297,3 +9298,53 @@ fn parse_create_aggregate_with_moving_aggregate_options() { _ => panic!("Expected CreateAggregate, got: {stmt:?}"), } } + +#[test] +fn parse_create_aggregate_star_args() { + let canonical = "CREATE AGGREGATE my_count (*) (SFUNC = int8inc, STYPE = INT8, INITCOND = '0')"; + let stmt = pg_and_generic().verified_stmt(canonical); + match stmt { + Statement::CreateAggregate(agg) => { + assert!(agg.star_args); + assert!(agg.args.is_empty()); + assert_eq!(agg.name.to_string(), "my_count"); + } + _ => panic!("Expected CreateAggregate, got: {stmt:?}"), + } + + pg_and_generic().one_statement_parses_to( + "CREATE AGGREGATE my_count ( * ) (SFUNC = int8inc, STYPE = INT8)", + "CREATE AGGREGATE my_count (*) (SFUNC = int8inc, STYPE = INT8)", + ); +} + +#[test] +fn parse_create_aggregate_named_and_variadic_args() { + let sql = + "CREATE AGGREGATE my_agg (input INT, VARIADIC tail TEXT) (SFUNC = my_sfunc, STYPE = INT)"; + let stmt = pg_and_generic().verified_stmt(sql); + match stmt { + Statement::CreateAggregate(agg) => { + assert_eq!(agg.args.len(), 2); + assert_eq!(agg.args[0].to_string(), "input INT"); + assert_eq!(agg.args[1].to_string(), "VARIADIC tail TEXT"); + } + _ => panic!("Expected CreateAggregate, got: {stmt:?}"), + } +} + +#[test] +fn parse_create_aggregate_additional_options() { + pg_and_generic().verified_stmt( + "CREATE AGGREGATE percentile (FLOAT8) (SFUNC = ordered_set_transition, STYPE = internal, FINALFUNC = percentile_final, FINALFUNC_MODIFY = READ_WRITE, HYPOTHETICAL)", + ); + pg_and_generic().verified_stmt( + "CREATE AGGREGATE my_min (INT) (SFUNC = my_sfunc, STYPE = INT, SSPACE = 128, SORTOP = <)", + ); + pg_and_generic().verified_stmt( + "CREATE AGGREGATE my_min2 (INT) (SFUNC = my_sfunc, STYPE = INT, SORTOP = pg_catalog.<)", + ); + pg_and_generic().verified_stmt( + "CREATE AGGREGATE my_sum (INT) (SFUNC = my_sfunc, STYPE = internal, COMBINEFUNC = my_combine, SERIALFUNC = my_serial, DESERIALFUNC = my_deserial)", + ); +} From 51f49773fddffac922092ffa68447afd96e44e1f Mon Sep 17 00:00:00 2001 From: Filipe Guerreiro Date: Sun, 14 Jun 2026 18:11:26 +0900 Subject: [PATCH 11/17] Address CREATE AGGREGATE review feedback - Replace em-dashes in CreateAggregateOption doc comments - Add FunctionParallel::as_str and reuse it in Display and the PARALLEL option - Support old-syntax BASETYPE option (parse and Display round-trip) - Clarify the CreateAggregate args field doc - Test the unknown-option error path and the old BASETYPE syntax --- src/ast/ddl.rs | 32 +++++++++++++++-------------- src/ast/mod.rs | 17 +++++++++++----- src/parser/mod.rs | 38 ++++++++++++++++++++++++++--------- tests/sqlparser_postgres.rs | 40 +++++++++++++++++++++++++++++++++++++ 4 files changed, 98 insertions(+), 29 deletions(-) diff --git a/src/ast/ddl.rs b/src/ast/ddl.rs index 09610dde5a..e3c0025a4f 100644 --- a/src/ast/ddl.rs +++ b/src/ast/ddl.rs @@ -5833,7 +5833,7 @@ pub struct CreateAggregate { pub or_replace: bool, /// The aggregate name (can be schema-qualified). pub name: ObjectName, - /// Input arguments. Empty for zero-argument aggregates or `(*)`. + /// Input arguments. Empty when `star_args` is true or when the argument list is empty. pub args: Vec, /// True if the argument list was the wildcard form `(*)` (used by /// zero-argument aggregates such as `count(*)`). @@ -5850,10 +5850,16 @@ impl fmt::Display for CreateAggregate { write!(f, " OR REPLACE")?; } write!(f, " AGGREGATE {}", self.name)?; - if self.star_args && self.args.is_empty() { - write!(f, " (*)")?; - } else { - write!(f, " ({})", display_comma_separated(&self.args))?; + let is_old_syntax = self + .options + .iter() + .any(|option| matches!(option, CreateAggregateOption::BaseType(_))); + if !is_old_syntax { + if self.star_args && self.args.is_empty() { + write!(f, " (*)")?; + } else { + write!(f, " ({})", display_comma_separated(&self.args))?; + } } write!(f, " ({})", display_comma_separated(&self.options)) } @@ -5880,7 +5886,7 @@ pub enum CreateAggregateOption { StateDataSize(u64), /// `FINALFUNC = final_function` FinalFunction(ObjectName), - /// `FINALFUNC_EXTRA` — pass extra dummy arguments to the final function. + /// `FINALFUNC_EXTRA`. Passes extra dummy arguments to the final function. FinalFunctionExtra, /// `FINALFUNC_MODIFY = { READ_ONLY | SHAREABLE | READ_WRITE }` FinalFunctionModify(AggregateModifyKind), @@ -5912,8 +5918,10 @@ pub enum CreateAggregateOption { SortOperator(ObjectName), /// `PARALLEL = { SAFE | RESTRICTED | UNSAFE }` Parallel(FunctionParallel), - /// `HYPOTHETICAL` — marks the aggregate as hypothetical-set. + /// `HYPOTHETICAL`. Marks the aggregate as hypothetical-set. Hypothetical, + /// `BASETYPE = base_type` (old aggregate syntax). + BaseType(DataType), } impl fmt::Display for CreateAggregateOption { @@ -5938,15 +5946,9 @@ impl fmt::Display for CreateAggregateOption { Self::MovingFinalFunctionModify(kind) => write!(f, "MFINALFUNC_MODIFY = {kind}"), Self::MovingInitialCondition(cond) => write!(f, "MINITCOND = {cond}"), Self::SortOperator(name) => write!(f, "SORTOP = {name}"), - Self::Parallel(parallel) => { - let kind = match parallel { - FunctionParallel::Safe => "SAFE", - FunctionParallel::Restricted => "RESTRICTED", - FunctionParallel::Unsafe => "UNSAFE", - }; - write!(f, "PARALLEL = {kind}") - } + Self::Parallel(parallel) => write!(f, "PARALLEL = {}", parallel.as_str()), Self::Hypothetical => write!(f, "HYPOTHETICAL"), + Self::BaseType(data_type) => write!(f, "BASETYPE = {data_type}"), } } } diff --git a/src/ast/mod.rs b/src/ast/mod.rs index ba6906360d..8102aee559 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -10069,16 +10069,23 @@ pub enum FunctionParallel { Safe, } -impl fmt::Display for FunctionParallel { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { +impl FunctionParallel { + /// Returns the bare keyword for this parallel mode, without the `PARALLEL` prefix. + pub fn as_str(&self) -> &'static str { match self { - FunctionParallel::Unsafe => write!(f, "PARALLEL UNSAFE"), - FunctionParallel::Restricted => write!(f, "PARALLEL RESTRICTED"), - FunctionParallel::Safe => write!(f, "PARALLEL SAFE"), + FunctionParallel::Unsafe => "UNSAFE", + FunctionParallel::Restricted => "RESTRICTED", + FunctionParallel::Safe => "SAFE", } } } +impl fmt::Display for FunctionParallel { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "PARALLEL {}", self.as_str()) + } +} + /// [BigQuery] Determinism specifier used in a UDF definition. /// /// [BigQuery]: https://cloud.google.com/bigquery/docs/reference/standard-sql/data-definition-language#syntax_11 diff --git a/src/parser/mod.rs b/src/parser/mod.rs index cdb3006c6c..a71f5e6798 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -7235,22 +7235,31 @@ impl<'a> Parser<'a> { ) -> Result { let name = self.parse_object_name(false)?; - // Argument list: `(input_arg [, ...])` or `(*)` for zero-arg. + // The old syntax has a single parenthesized list that carries both + // `BASETYPE` and the remaining options. The modern syntax has a + // separate `(input_arg [, ...])` argument list followed by the + // options list. They are distinguished by whether a second + // parenthesized list follows the first. + let checkpoint = self.index; self.expect_token(&Token::LParen)?; - let star_args = self.consume_token(&Token::Mul); - let args = if star_args || self.peek_token().token == Token::RParen { + let mut star_args = self.consume_token(&Token::Mul); + let mut args = if star_args || self.peek_token().token == Token::RParen { vec![] } else { self.parse_comma_separated(Parser::parse_function_arg)? }; self.expect_token(&Token::RParen)?; - // Options block: `( SFUNC = ..., STYPE = ..., ... )`. - self.expect_token(&Token::LParen)?; - let options = self.parse_comma_separated(|parser| { - let key = parser.parse_identifier()?; - parser.parse_create_aggregate_option(&key.value.to_uppercase()) - })?; + // The first list is the modern argument-type list only when a second + // list follows. Otherwise it was the old-syntax options list, so rewind, + // discard the speculatively parsed args, and re-parse it as options. + if !self.consume_token(&Token::LParen) { + self.index = checkpoint; + args = vec![]; + star_args = false; + self.expect_token(&Token::LParen)?; + } + let options = self.parse_comma_separated(Parser::parse_create_aggregate_option_entry)?; self.expect_token(&Token::RParen)?; Ok(CreateAggregate { @@ -7275,6 +7284,13 @@ impl<'a> Parser<'a> { } } + fn parse_create_aggregate_option_entry( + &mut self, + ) -> Result { + let key = self.parse_identifier()?; + self.parse_create_aggregate_option(&key.value.to_uppercase()) + } + fn parse_create_aggregate_option( &mut self, key: &str, @@ -7387,6 +7403,10 @@ impl<'a> Parser<'a> { )) } "HYPOTHETICAL" => Ok(CreateAggregateOption::Hypothetical), + "BASETYPE" => { + self.expect_token(&Token::Eq)?; + Ok(CreateAggregateOption::BaseType(self.parse_data_type()?)) + } other => Err(ParserError::ParserError(format!( "Unknown CREATE AGGREGATE option: {other}" ))), diff --git a/tests/sqlparser_postgres.rs b/tests/sqlparser_postgres.rs index 8c7898db9d..de06a7f2cc 100644 --- a/tests/sqlparser_postgres.rs +++ b/tests/sqlparser_postgres.rs @@ -9348,3 +9348,43 @@ fn parse_create_aggregate_additional_options() { "CREATE AGGREGATE my_sum (INT) (SFUNC = my_sfunc, STYPE = internal, COMBINEFUNC = my_combine, SERIALFUNC = my_serial, DESERIALFUNC = my_deserial)", ); } + +#[test] +fn parse_create_aggregate_old_syntax_basetype() { + let stmt = pg_and_generic() + .verified_stmt("CREATE AGGREGATE my_avg (BASETYPE = INT, SFUNC = my_sfunc, STYPE = INT)"); + match stmt { + Statement::CreateAggregate(agg) => { + assert!(agg.args.is_empty()); + assert!(!agg.star_args); + assert_eq!(agg.options.len(), 3); + assert_eq!( + agg.options[0], + CreateAggregateOption::BaseType(DataType::Int(None)) + ); + } + _ => panic!("Expected CreateAggregate, got: {stmt:?}"), + } + + let stmt = pg_and_generic() + .verified_stmt("CREATE AGGREGATE my_avg (SFUNC = my_sfunc, BASETYPE = INT, STYPE = INT)"); + match stmt { + Statement::CreateAggregate(agg) => { + assert!(agg.args.is_empty()); + assert!(!agg.star_args); + assert_eq!(agg.options.len(), 3); + assert_eq!( + agg.options[1], + CreateAggregateOption::BaseType(DataType::Int(None)) + ); + } + _ => panic!("Expected CreateAggregate, got: {stmt:?}"), + } +} + +#[test] +fn parse_create_aggregate_unknown_option() { + assert!(pg_and_generic() + .parse_sql_statements("CREATE AGGREGATE foo (INT) (UNKNOWN_OPTION = bar)") + .is_err()); +} From 6b4f1bb5983b04b1a8f35e365736549230d99ce3 Mon Sep 17 00:00:00 2001 From: Filipe Guerreiro Date: Mon, 13 Jul 2026 08:37:47 +0900 Subject: [PATCH 12/17] Reformat ast re-export list with cargo fmt --- src/ast/mod.rs | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/ast/mod.rs b/src/ast/mod.rs index b2489f47bf..bee59db7b4 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -72,18 +72,18 @@ pub use self::ddl::{ AlterTypeAddValuePosition, AlterTypeOperation, AlterTypeRename, AlterTypeRenameValue, ClusteredBy, ColumnDef, ColumnOption, ColumnOptionDef, ColumnOptions, ColumnPolicy, ColumnPolicyProperty, ConstraintCharacteristics, CreateAggregate, CreateAggregateOption, - CreateCollation, CreateCollationDefinition, CreateConnector, - CreateDomain, CreateExtension, CreateFunction, CreateIndex, CreateOperator, - CreateOperatorClass, CreateOperatorFamily, CreatePolicy, CreatePolicyCommand, CreatePolicyType, - CreateTable, CreateTextSearch, CreateTrigger, CreateView, Deduplicate, DeferrableInitial, - DistStyle, DropBehavior, DropExtension, DropFunction, DropOperator, DropOperatorClass, - DropOperatorFamily, DropOperatorSignature, DropPolicy, DropTrigger, ForValues, - FunctionReturnType, GeneratedAs, GeneratedExpressionMode, IdentityParameters, IdentityProperty, - IdentityPropertyFormatKind, IdentityPropertyKind, IdentityPropertyOrder, IndexColumn, - IndexOption, IndexType, KeyOrIndexDisplay, Msck, NullsDistinctOption, OperatorArgTypes, - OperatorClassItem, OperatorFamilyDropItem, OperatorFamilyItem, OperatorOption, OperatorPurpose, - Owner, Partition, PartitionBoundValue, ProcedureParam, ReferentialAction, RenameTableNameKind, - ReplicaIdentity, TagsColumnOption, TextSearchObjectType, TriggerObjectKind, Truncate, + CreateCollation, CreateCollationDefinition, CreateConnector, CreateDomain, CreateExtension, + CreateFunction, CreateIndex, CreateOperator, CreateOperatorClass, CreateOperatorFamily, + CreatePolicy, CreatePolicyCommand, CreatePolicyType, CreateTable, CreateTextSearch, + CreateTrigger, CreateView, Deduplicate, DeferrableInitial, DistStyle, DropBehavior, + DropExtension, DropFunction, DropOperator, DropOperatorClass, DropOperatorFamily, + DropOperatorSignature, DropPolicy, DropTrigger, ForValues, FunctionReturnType, GeneratedAs, + GeneratedExpressionMode, IdentityParameters, IdentityProperty, IdentityPropertyFormatKind, + IdentityPropertyKind, IdentityPropertyOrder, IndexColumn, IndexOption, IndexType, + KeyOrIndexDisplay, Msck, NullsDistinctOption, OperatorArgTypes, OperatorClassItem, + OperatorFamilyDropItem, OperatorFamilyItem, OperatorOption, OperatorPurpose, Owner, Partition, + PartitionBoundValue, ProcedureParam, ReferentialAction, RenameTableNameKind, ReplicaIdentity, + TagsColumnOption, TextSearchObjectType, TriggerObjectKind, Truncate, UserDefinedTypeCompositeAttributeDef, UserDefinedTypeInternalLength, UserDefinedTypeRangeOption, UserDefinedTypeRepresentation, UserDefinedTypeSqlDefinitionOption, UserDefinedTypeStorage, ViewColumnDef, WithData, From 73f9d6818dc46564f4c942a91e4e2e2dae84b968 Mon Sep 17 00:00:00 2001 From: Filipe Guerreiro Date: Fri, 24 Jul 2026 10:30:05 +0900 Subject: [PATCH 13/17] PostgreSQL: fix CREATE AGGREGATE round-trip and legacy-syntax parsing - record the syntax form in the AST. `CreateAggregateArgs` (Legacy/Star/List) replaces `Vec` plus a `star_args` flag, so Display no longer infers the legacy form by scanning the options for BASETYPE. That scan round-tripped `foo (SFUNC = f, STYPE = INT)` back out with an added empty argument list, and let `(*)` coexist with a non-empty argument list. - parse the modern argument list with `maybe_parse` instead of a hand-rolled rewind. An error inside the speculative parse propagated instead of falling back, so legacy statements whose option values cannot start an argument definition (`SORTOP = <`) failed to parse. - promote the option keys to keywords, matching the CREATE OPERATOR parsing next to it. Quoted keys and duplicate options are now rejected, and the errors carry a source location. - fold the three copies of PARALLEL { SAFE | RESTRICTED | UNSAFE } parsing into one helper. --- src/ast/ddl.rs | 37 +++-- src/ast/mod.rs | 26 +-- src/keywords.rs | 23 +++ src/parser/mod.rs | 317 ++++++++++++++++-------------------- tests/sqlparser_postgres.rs | 86 ++++++++-- 5 files changed, 265 insertions(+), 224 deletions(-) diff --git a/src/ast/ddl.rs b/src/ast/ddl.rs index b70d74c328..3522d1e1a8 100644 --- a/src/ast/ddl.rs +++ b/src/ast/ddl.rs @@ -6005,11 +6005,8 @@ pub struct CreateAggregate { pub or_replace: bool, /// The aggregate name (can be schema-qualified). pub name: ObjectName, - /// Input arguments. Empty when `star_args` is true or when the argument list is empty. - pub args: Vec, - /// True if the argument list was the wildcard form `(*)` (used by - /// zero-argument aggregates such as `count(*)`). - pub star_args: bool, + /// The argument list preceding the options list. + pub args: CreateAggregateArgs, /// The options listed inside the required parentheses after the argument /// list (e.g. `SFUNC`, `STYPE`, `FINALFUNC`, `PARALLEL`, …). pub options: Vec, @@ -6022,21 +6019,31 @@ impl fmt::Display for CreateAggregate { write!(f, " OR REPLACE")?; } write!(f, " AGGREGATE {}", self.name)?; - let is_old_syntax = self - .options - .iter() - .any(|option| matches!(option, CreateAggregateOption::BaseType(_))); - if !is_old_syntax { - if self.star_args && self.args.is_empty() { - write!(f, " (*)")?; - } else { - write!(f, " ({})", display_comma_separated(&self.args))?; - } + match &self.args { + CreateAggregateArgs::Legacy => {} + CreateAggregateArgs::Star => write!(f, " (*)")?, + CreateAggregateArgs::List(args) => write!(f, " ({})", display_comma_separated(args))?, } write!(f, " ({})", display_comma_separated(&self.options)) } } +/// The argument list of a [`CreateAggregate`] statement. +#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)] +#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] +#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))] +pub enum CreateAggregateArgs { + /// No argument list at all: the pre-8.2 syntax that instead carries a + /// `BASETYPE` entry in the options list. + Legacy, + /// The wildcard form `(*)`, used by zero-argument aggregates such as + /// `count(*)`. + Star, + /// An explicit argument list, possibly empty: `()`, `(NUMERIC)`, + /// `(input INT, VARIADIC tail TEXT)`. + List(Vec), +} + impl From for crate::ast::Statement { fn from(v: CreateAggregate) -> Self { crate::ast::Statement::CreateAggregate(v) diff --git a/src/ast/mod.rs b/src/ast/mod.rs index add685519e..cf9ffbfc1f 100644 --- a/src/ast/mod.rs +++ b/src/ast/mod.rs @@ -71,19 +71,19 @@ pub use self::ddl::{ AlterTextSearch, AlterTextSearchOperation, AlterTextSearchOption, AlterType, AlterTypeAddValue, AlterTypeAddValuePosition, AlterTypeOperation, AlterTypeRename, AlterTypeRenameValue, ClusteredBy, ColumnDef, ColumnOption, ColumnOptionDef, ColumnOptions, ColumnPolicy, - ColumnPolicyProperty, ConstraintCharacteristics, CreateAggregate, CreateAggregateOption, - CreateCollation, CreateCollationDefinition, CreateConnector, CreateDomain, CreateExtension, - CreateFunction, CreateIndex, CreateOperator, CreateOperatorClass, CreateOperatorFamily, - CreatePolicy, CreatePolicyCommand, CreatePolicyType, CreateTable, CreateTextSearch, - CreateTrigger, CreateView, Deduplicate, DeferrableInitial, DistStyle, DropBehavior, - DropExtension, DropFunction, DropOperator, DropOperatorClass, DropOperatorFamily, - DropOperatorSignature, DropPolicy, DropTrigger, ForValues, FunctionReturnType, GeneratedAs, - GeneratedExpressionMode, IdentityParameters, IdentityProperty, IdentityPropertyFormatKind, - IdentityPropertyKind, IdentityPropertyOrder, IndexColumn, IndexOption, IndexType, - KeyOrIndexDisplay, Msck, NullsDistinctOption, OperatorArgTypes, OperatorClassItem, - OperatorFamilyDropItem, OperatorFamilyItem, OperatorOption, OperatorPurpose, Owner, Partition, - PartitionBoundValue, ProcedureParam, ReferentialAction, RenameTableNameKind, ReplicaIdentity, - TagsColumnOption, TextSearchObjectType, TriggerObjectKind, Truncate, + ColumnPolicyProperty, ConstraintCharacteristics, CreateAggregate, CreateAggregateArgs, + CreateAggregateOption, CreateCollation, CreateCollationDefinition, CreateConnector, + CreateDomain, CreateExtension, CreateFunction, CreateIndex, CreateOperator, + CreateOperatorClass, CreateOperatorFamily, CreatePolicy, CreatePolicyCommand, CreatePolicyType, + CreateTable, CreateTextSearch, CreateTrigger, CreateView, Deduplicate, DeferrableInitial, + DistStyle, DropBehavior, DropExtension, DropFunction, DropOperator, DropOperatorClass, + DropOperatorFamily, DropOperatorSignature, DropPolicy, DropTrigger, ForValues, + FunctionReturnType, GeneratedAs, GeneratedExpressionMode, IdentityParameters, IdentityProperty, + IdentityPropertyFormatKind, IdentityPropertyKind, IdentityPropertyOrder, IndexColumn, + IndexOption, IndexType, KeyOrIndexDisplay, Msck, NullsDistinctOption, OperatorArgTypes, + OperatorClassItem, OperatorFamilyDropItem, OperatorFamilyItem, OperatorOption, OperatorPurpose, + Owner, Partition, PartitionBoundValue, ProcedureParam, ReferentialAction, RenameTableNameKind, + ReplicaIdentity, TagsColumnOption, TextSearchObjectType, TriggerObjectKind, Truncate, UserDefinedTypeCompositeAttributeDef, UserDefinedTypeInternalLength, UserDefinedTypeRangeOption, UserDefinedTypeRepresentation, UserDefinedTypeSqlDefinitionOption, UserDefinedTypeStorage, ViewColumnDef, WithData, diff --git a/src/keywords.rs b/src/keywords.rs index c2d0a47ec0..b7795cf763 100644 --- a/src/keywords.rs +++ b/src/keywords.rs @@ -149,6 +149,7 @@ define_keywords!( BACKUP, BACKWARD, BASE64, + BASETYPE, BASE_LOCATION, BEFORE, BEGIN, @@ -236,6 +237,7 @@ define_keywords!( COLUMN, COLUMNS, COLUMNSTORE, + COMBINEFUNC, COMMENT, COMMIT, COMMITTED, @@ -332,6 +334,7 @@ define_keywords!( DEREF, DESC, DESCRIBE, + DESERIALFUNC, DETACH, DETAIL, DETERMINISTIC, @@ -428,6 +431,9 @@ define_keywords!( FILL, FILTER, FINAL, + FINALFUNC, + FINALFUNC_EXTRA, + FINALFUNC_MODIFY, FIRST, FIRST_VALUE, FIXEDSTRING, @@ -494,6 +500,7 @@ define_keywords!( HOUR, HOURS, HUGEINT, + HYPOTHETICAL, IAM_ROLE, ICEBERG, ID, @@ -518,6 +525,7 @@ define_keywords!( INDICATOR, INHERIT, INHERITS, + INITCOND, INITIALIZE, INITIALLY, INNER, @@ -646,6 +654,9 @@ define_keywords!( METRIC, METRICS, MFA, + MFINALFUNC, + MFINALFUNC_EXTRA, + MFINALFUNC_MODIFY, MICROSECOND, MICROSECONDS, MILLENIUM, @@ -653,10 +664,12 @@ define_keywords!( MILLISECOND, MILLISECONDS, MIN, + MINITCOND, MINUS, MINUTE, MINUTES, MINVALUE, + MINVFUNC, MIN_ROWS, MOD, MODE, @@ -668,6 +681,9 @@ define_keywords!( MONTH, MONTHS, MSCK, + MSFUNC, + MSSPACE, + MSTYPE, MULTIRANGE_TYPE_NAME, MULTISET, MUTATION, @@ -842,6 +858,7 @@ define_keywords!( READ, READS, READ_ONLY, + READ_WRITE, REAL, RECEIVE, RECLUSTER, @@ -944,6 +961,7 @@ define_keywords!( SEQUENCES, SERDE, SERDEPROPERTIES, + SERIALFUNC, SERIALIZABLE, SERVER, SERVICE, @@ -954,7 +972,9 @@ define_keywords!( SETOF, SETS, SETTINGS, + SFUNC, SHARE, + SHAREABLE, SHARED, SHARING, SHOW, @@ -970,6 +990,7 @@ define_keywords!( SORT, SORTED, SORTKEY, + SORTOP, SOURCE, SPATIAL, SPECIFIC, @@ -986,6 +1007,7 @@ define_keywords!( SQL_SMALL_RESULT, SQRT, SRID, + SSPACE, STABLE, STAGE, START, @@ -1012,6 +1034,7 @@ define_keywords!( STRICT, STRING, STRUCT, + STYPE, SUBMULTISET, SUBSCRIPT, SUBSTR, diff --git a/src/parser/mod.rs b/src/parser/mod.rs index 3771d3ed65..c4fa9e07da 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -5902,16 +5902,7 @@ impl<'a> Parser<'a> { body.called_on_null = Some(FunctionCalledOnNull::Strict); } else if self.parse_keyword(Keyword::PARALLEL) { ensure_not_set(&body.parallel, "PARALLEL { UNSAFE | RESTRICTED | SAFE }")?; - if self.parse_keyword(Keyword::UNSAFE) { - body.parallel = Some(FunctionParallel::Unsafe); - } else if self.parse_keyword(Keyword::RESTRICTED) { - body.parallel = Some(FunctionParallel::Restricted); - } else if self.parse_keyword(Keyword::SAFE) { - body.parallel = Some(FunctionParallel::Safe); - } else { - return self - .expected_ref("one of UNSAFE | RESTRICTED | SAFE", self.peek_token_ref()); - } + body.parallel = Some(self.parse_function_parallel()?); } else if self.parse_keyword(Keyword::SECURITY) { ensure_not_set(&body.security, "SECURITY { DEFINER | INVOKER }")?; if self.parse_keyword(Keyword::DEFINER) { @@ -7436,198 +7427,176 @@ impl<'a> Parser<'a> { ) -> Result { let name = self.parse_object_name(false)?; - // The old syntax has a single parenthesized list that carries both + // The keys accepted inside the options list. + const OPTIONS: &[Keyword] = &[ + Keyword::SFUNC, + Keyword::STYPE, + Keyword::SSPACE, + Keyword::FINALFUNC, + Keyword::FINALFUNC_EXTRA, + Keyword::FINALFUNC_MODIFY, + Keyword::COMBINEFUNC, + Keyword::SERIALFUNC, + Keyword::DESERIALFUNC, + Keyword::INITCOND, + Keyword::MSFUNC, + Keyword::MINVFUNC, + Keyword::MSTYPE, + Keyword::MSSPACE, + Keyword::MFINALFUNC, + Keyword::MFINALFUNC_EXTRA, + Keyword::MFINALFUNC_MODIFY, + Keyword::MINITCOND, + Keyword::SORTOP, + Keyword::PARALLEL, + Keyword::HYPOTHETICAL, + Keyword::BASETYPE, + ]; + + // The legacy syntax has a single parenthesized list that carries both // `BASETYPE` and the remaining options. The modern syntax has a - // separate `(input_arg [, ...])` argument list followed by the - // options list. They are distinguished by whether a second - // parenthesized list follows the first. - let checkpoint = self.index; - self.expect_token(&Token::LParen)?; - let mut star_args = self.consume_token(&Token::Mul); - let mut args = if star_args || self.peek_token().token == Token::RParen { - vec![] - } else { - self.parse_comma_separated(Parser::parse_function_arg)? + // separate `(input_arg [, ...])` argument list followed by the options + // list. They are only distinguishable by whether a second parenthesized + // list follows the first, so parse the argument list speculatively and + // fall back to the legacy form when no options list follows it. + let args = match self.maybe_parse(|parser| { + let args = parser.parse_create_aggregate_args()?; + parser.expect_token(&Token::LParen)?; + Ok(args) + })? { + Some(args) => args, + None => { + self.expect_token(&Token::LParen)?; + CreateAggregateArgs::Legacy + } }; - self.expect_token(&Token::RParen)?; - // The first list is the modern argument-type list only when a second - // list follows. Otherwise it was the old-syntax options list, so rewind, - // discard the speculatively parsed args, and re-parse it as options. - if !self.consume_token(&Token::LParen) { - self.index = checkpoint; - args = vec![]; - star_args = false; - self.expect_token(&Token::LParen)?; - } - let options = self.parse_comma_separated(Parser::parse_create_aggregate_option_entry)?; + let mut seen: Vec = Vec::new(); + let options = self.parse_comma_separated(|parser| { + let token = parser.peek_token(); + let keyword = parser.expect_one_of_keywords(OPTIONS)?; + if seen.contains(&keyword) { + return parser.expected("no duplicate CREATE AGGREGATE option", token); + } + seen.push(keyword); + parser.parse_create_aggregate_option(keyword) + })?; self.expect_token(&Token::RParen)?; Ok(CreateAggregate { or_replace, name, args, - star_args, options, }) } - /// Parse `PARALLEL` qualifier value: `{ SAFE | RESTRICTED | UNSAFE }`. - fn parse_function_parallel(&mut self) -> Result { - if self.parse_keyword(Keyword::SAFE) { - Ok(FunctionParallel::Safe) - } else if self.parse_keyword(Keyword::RESTRICTED) { - Ok(FunctionParallel::Restricted) - } else if self.parse_keyword(Keyword::UNSAFE) { - Ok(FunctionParallel::Unsafe) + /// Parse the argument list of a `CREATE AGGREGATE`: `(*)` or `(arg [, ...])`. + fn parse_create_aggregate_args(&mut self) -> Result { + self.expect_token(&Token::LParen)?; + let args = if self.consume_token(&Token::Mul) { + CreateAggregateArgs::Star } else { - self.expected_ref("one of SAFE | RESTRICTED | UNSAFE", self.peek_token_ref()) - } + CreateAggregateArgs::List( + self.parse_comma_separated0(Parser::parse_function_arg, Token::RParen)?, + ) + }; + self.expect_token(&Token::RParen)?; + Ok(args) } - fn parse_create_aggregate_option_entry( - &mut self, - ) -> Result { - let key = self.parse_identifier()?; - self.parse_create_aggregate_option(&key.value.to_uppercase()) + /// Parse `PARALLEL` qualifier value: `{ SAFE | RESTRICTED | UNSAFE }`. + fn parse_function_parallel(&mut self) -> Result { + match self.expect_one_of_keywords(&[Keyword::SAFE, Keyword::RESTRICTED, Keyword::UNSAFE])? { + Keyword::SAFE => Ok(FunctionParallel::Safe), + Keyword::RESTRICTED => Ok(FunctionParallel::Restricted), + Keyword::UNSAFE => Ok(FunctionParallel::Unsafe), + _ => self.expected_ref("one of SAFE | RESTRICTED | UNSAFE", self.peek_token_ref()), + } } + /// Parse the value of a single `CREATE AGGREGATE` option, given its + /// already-consumed key. fn parse_create_aggregate_option( &mut self, - key: &str, + keyword: Keyword, ) -> Result { - match key { - "SFUNC" => { - self.expect_token(&Token::Eq)?; - Ok(CreateAggregateOption::StateTransitionFunction( - self.parse_object_name(false)?, - )) - } - "STYPE" => { - self.expect_token(&Token::Eq)?; - Ok(CreateAggregateOption::StateDataType( - self.parse_data_type()?, - )) - } - "SSPACE" => { - self.expect_token(&Token::Eq)?; - let size = self.parse_literal_uint()?; - Ok(CreateAggregateOption::StateDataSize(size)) - } - "FINALFUNC" => { - self.expect_token(&Token::Eq)?; - Ok(CreateAggregateOption::FinalFunction( - self.parse_object_name(false)?, - )) - } - "FINALFUNC_EXTRA" => Ok(CreateAggregateOption::FinalFunctionExtra), - "FINALFUNC_MODIFY" => { - self.expect_token(&Token::Eq)?; - Ok(CreateAggregateOption::FinalFunctionModify( - self.parse_aggregate_modify_kind()?, - )) - } - "COMBINEFUNC" => { - self.expect_token(&Token::Eq)?; - Ok(CreateAggregateOption::CombineFunction( - self.parse_object_name(false)?, - )) - } - "SERIALFUNC" => { - self.expect_token(&Token::Eq)?; - Ok(CreateAggregateOption::SerialFunction( - self.parse_object_name(false)?, - )) + // Every option but the three valueless flags is spelled `KEY = value`. + let is_flag = matches!( + keyword, + Keyword::FINALFUNC_EXTRA | Keyword::MFINALFUNC_EXTRA | Keyword::HYPOTHETICAL + ); + if !is_flag { + self.expect_token(&Token::Eq)?; + } + + Ok(match keyword { + Keyword::FINALFUNC_EXTRA => CreateAggregateOption::FinalFunctionExtra, + Keyword::MFINALFUNC_EXTRA => CreateAggregateOption::MovingFinalFunctionExtra, + Keyword::HYPOTHETICAL => CreateAggregateOption::Hypothetical, + Keyword::SFUNC => { + CreateAggregateOption::StateTransitionFunction(self.parse_object_name(false)?) } - "DESERIALFUNC" => { - self.expect_token(&Token::Eq)?; - Ok(CreateAggregateOption::DeserialFunction( - self.parse_object_name(false)?, - )) + Keyword::STYPE => CreateAggregateOption::StateDataType(self.parse_data_type()?), + Keyword::SSPACE => CreateAggregateOption::StateDataSize(self.parse_literal_uint()?), + Keyword::FINALFUNC => { + CreateAggregateOption::FinalFunction(self.parse_object_name(false)?) } - "INITCOND" => { - self.expect_token(&Token::Eq)?; - Ok(CreateAggregateOption::InitialCondition(self.parse_value()?)) + Keyword::FINALFUNC_MODIFY => { + CreateAggregateOption::FinalFunctionModify(self.parse_aggregate_modify_kind()?) } - "MSFUNC" => { - self.expect_token(&Token::Eq)?; - Ok(CreateAggregateOption::MovingStateTransitionFunction( - self.parse_object_name(false)?, - )) + Keyword::COMBINEFUNC => { + CreateAggregateOption::CombineFunction(self.parse_object_name(false)?) } - "MINVFUNC" => { - self.expect_token(&Token::Eq)?; - Ok(CreateAggregateOption::MovingInverseTransitionFunction( - self.parse_object_name(false)?, - )) + Keyword::SERIALFUNC => { + CreateAggregateOption::SerialFunction(self.parse_object_name(false)?) } - "MSTYPE" => { - self.expect_token(&Token::Eq)?; - Ok(CreateAggregateOption::MovingStateDataType( - self.parse_data_type()?, - )) - } - "MSSPACE" => { - self.expect_token(&Token::Eq)?; - let size = self.parse_literal_uint()?; - Ok(CreateAggregateOption::MovingStateDataSize(size)) + Keyword::DESERIALFUNC => { + CreateAggregateOption::DeserialFunction(self.parse_object_name(false)?) } - "MFINALFUNC" => { - self.expect_token(&Token::Eq)?; - Ok(CreateAggregateOption::MovingFinalFunction( - self.parse_object_name(false)?, - )) - } - "MFINALFUNC_EXTRA" => Ok(CreateAggregateOption::MovingFinalFunctionExtra), - "MFINALFUNC_MODIFY" => { - self.expect_token(&Token::Eq)?; - Ok(CreateAggregateOption::MovingFinalFunctionModify( - self.parse_aggregate_modify_kind()?, - )) + Keyword::INITCOND => CreateAggregateOption::InitialCondition(self.parse_value()?), + Keyword::MSFUNC => { + CreateAggregateOption::MovingStateTransitionFunction(self.parse_object_name(false)?) } - "MINITCOND" => { - self.expect_token(&Token::Eq)?; - Ok(CreateAggregateOption::MovingInitialCondition( - self.parse_value()?, - )) + Keyword::MINVFUNC => CreateAggregateOption::MovingInverseTransitionFunction( + self.parse_object_name(false)?, + ), + Keyword::MSTYPE => CreateAggregateOption::MovingStateDataType(self.parse_data_type()?), + Keyword::MSSPACE => { + CreateAggregateOption::MovingStateDataSize(self.parse_literal_uint()?) } - "SORTOP" => { - self.expect_token(&Token::Eq)?; - Ok(CreateAggregateOption::SortOperator( - self.parse_operator_name()?, - )) + Keyword::MFINALFUNC => { + CreateAggregateOption::MovingFinalFunction(self.parse_object_name(false)?) } - "PARALLEL" => { - self.expect_token(&Token::Eq)?; - Ok(CreateAggregateOption::Parallel( - self.parse_function_parallel()?, - )) + Keyword::MFINALFUNC_MODIFY => CreateAggregateOption::MovingFinalFunctionModify( + self.parse_aggregate_modify_kind()?, + ), + Keyword::MINITCOND => { + CreateAggregateOption::MovingInitialCondition(self.parse_value()?) } - "HYPOTHETICAL" => Ok(CreateAggregateOption::Hypothetical), - "BASETYPE" => { - self.expect_token(&Token::Eq)?; - Ok(CreateAggregateOption::BaseType(self.parse_data_type()?)) + Keyword::SORTOP => CreateAggregateOption::SortOperator(self.parse_operator_name()?), + Keyword::PARALLEL => CreateAggregateOption::Parallel(self.parse_function_parallel()?), + Keyword::BASETYPE => CreateAggregateOption::BaseType(self.parse_data_type()?), + _ => { + return self.expected_ref("a CREATE AGGREGATE option", self.peek_token_ref()); } - other => Err(ParserError::ParserError(format!( - "Unknown CREATE AGGREGATE option: {other}" - ))), - } + }) } + /// Parse the value of `FINALFUNC_MODIFY` / `MFINALFUNC_MODIFY`. fn parse_aggregate_modify_kind(&mut self) -> Result { - let token = self.next_token(); - match &token.token { - Token::Word(word) => match word.value.to_uppercase().as_str() { - "READ_ONLY" => Ok(AggregateModifyKind::ReadOnly), - "SHAREABLE" => Ok(AggregateModifyKind::Shareable), - "READ_WRITE" => Ok(AggregateModifyKind::ReadWrite), - other => Err(ParserError::ParserError(format!( - "Expected READ_ONLY, SHAREABLE, or READ_WRITE, got: {other}" - ))), - }, - other => Err(ParserError::ParserError(format!( - "Expected READ_ONLY, SHAREABLE, or READ_WRITE, got: {other:?}" - ))), + match self.expect_one_of_keywords(&[ + Keyword::READ_ONLY, + Keyword::SHAREABLE, + Keyword::READ_WRITE, + ])? { + Keyword::READ_ONLY => Ok(AggregateModifyKind::ReadOnly), + Keyword::SHAREABLE => Ok(AggregateModifyKind::Shareable), + Keyword::READ_WRITE => Ok(AggregateModifyKind::ReadWrite), + _ => self.expected_ref( + "one of READ_ONLY | SHAREABLE | READ_WRITE", + self.peek_token_ref(), + ), } } @@ -11504,17 +11473,9 @@ impl<'a> Parser<'a> { security, }) } else if self.parse_keyword(Keyword::PARALLEL) { - let parallel = if self.parse_keyword(Keyword::UNSAFE) { - FunctionParallel::Unsafe - } else if self.parse_keyword(Keyword::RESTRICTED) { - FunctionParallel::Restricted - } else if self.parse_keyword(Keyword::SAFE) { - FunctionParallel::Safe - } else { - return self - .expected_ref("one of UNSAFE | RESTRICTED | SAFE", self.peek_token_ref()); - }; - Some(AlterFunctionAction::Parallel(parallel)) + Some(AlterFunctionAction::Parallel( + self.parse_function_parallel()?, + )) } else if self.parse_keyword(Keyword::COST) { Some(AlterFunctionAction::Cost(self.parse_number()?)) } else if self.parse_keyword(Keyword::ROWS) { diff --git a/tests/sqlparser_postgres.rs b/tests/sqlparser_postgres.rs index 932c82c77e..48a3bf853f 100644 --- a/tests/sqlparser_postgres.rs +++ b/tests/sqlparser_postgres.rs @@ -9539,6 +9539,15 @@ fn parse_lock_table() { } } +/// The rendered argument list of a `CREATE AGGREGATE`, which must not be the +/// legacy or wildcard form. +fn aggregate_args(args: &CreateAggregateArgs) -> Vec { + match args { + CreateAggregateArgs::List(args) => args.iter().map(ToString::to_string).collect(), + other => panic!("Expected an argument list, got: {other:?}"), + } +} + #[test] fn parse_create_aggregate_basic() { let sql = "CREATE AGGREGATE myavg (NUMERIC) (SFUNC = numeric_avg_accum, STYPE = internal, FINALFUNC = numeric_avg, INITCOND = '0')"; @@ -9546,10 +9555,8 @@ fn parse_create_aggregate_basic() { match stmt { Statement::CreateAggregate(agg) => { assert!(!agg.or_replace); - assert!(!agg.star_args); assert_eq!(agg.name.to_string(), "myavg"); - assert_eq!(agg.args.len(), 1); - assert_eq!(agg.args[0].to_string(), "NUMERIC"); + assert_eq!(aggregate_args(&agg.args), ["NUMERIC"]); assert_eq!(agg.options.len(), 4); assert_eq!(agg.options[0].to_string(), "SFUNC = numeric_avg_accum"); assert_eq!(agg.options[1].to_string(), "STYPE = internal"); @@ -9568,7 +9575,7 @@ fn parse_create_aggregate_or_replace_with_parallel() { Statement::CreateAggregate(agg) => { assert!(agg.or_replace); assert_eq!(agg.name.to_string(), "sum2"); - assert_eq!(agg.args.len(), 2); + assert_eq!(aggregate_args(&agg.args), ["INT4", "INT4"]); assert_eq!(agg.options.len(), 3); assert_eq!(agg.options[2].to_string(), "PARALLEL = SAFE"); } @@ -9584,7 +9591,7 @@ fn parse_create_aggregate_with_moving_aggregate_options() { Statement::CreateAggregate(agg) => { assert!(!agg.or_replace); assert_eq!(agg.name.to_string(), "moving_sum"); - assert_eq!(agg.args.len(), 1); + assert_eq!(aggregate_args(&agg.args), ["FLOAT8"]); assert_eq!(agg.options.len(), 7); assert_eq!(agg.options[4].to_string(), "MSTYPE = FLOAT8"); assert_eq!(agg.options[5].to_string(), "MFINALFUNC_EXTRA"); @@ -9600,8 +9607,7 @@ fn parse_create_aggregate_star_args() { let stmt = pg_and_generic().verified_stmt(canonical); match stmt { Statement::CreateAggregate(agg) => { - assert!(agg.star_args); - assert!(agg.args.is_empty()); + assert_eq!(agg.args, CreateAggregateArgs::Star); assert_eq!(agg.name.to_string(), "my_count"); } _ => panic!("Expected CreateAggregate, got: {stmt:?}"), @@ -9613,6 +9619,18 @@ fn parse_create_aggregate_star_args() { ); } +#[test] +fn parse_create_aggregate_empty_args() { + let stmt = pg_and_generic() + .verified_stmt("CREATE AGGREGATE my_agg () (SFUNC = my_sfunc, STYPE = INT)"); + match stmt { + Statement::CreateAggregate(agg) => { + assert_eq!(agg.args, CreateAggregateArgs::List(vec![])); + } + _ => panic!("Expected CreateAggregate, got: {stmt:?}"), + } +} + #[test] fn parse_create_aggregate_named_and_variadic_args() { let sql = @@ -9620,9 +9638,10 @@ fn parse_create_aggregate_named_and_variadic_args() { let stmt = pg_and_generic().verified_stmt(sql); match stmt { Statement::CreateAggregate(agg) => { - assert_eq!(agg.args.len(), 2); - assert_eq!(agg.args[0].to_string(), "input INT"); - assert_eq!(agg.args[1].to_string(), "VARIADIC tail TEXT"); + assert_eq!( + aggregate_args(&agg.args), + ["input INT", "VARIADIC tail TEXT"] + ); } _ => panic!("Expected CreateAggregate, got: {stmt:?}"), } @@ -9642,16 +9661,18 @@ fn parse_create_aggregate_additional_options() { pg_and_generic().verified_stmt( "CREATE AGGREGATE my_sum (INT) (SFUNC = my_sfunc, STYPE = internal, COMBINEFUNC = my_combine, SERIALFUNC = my_serial, DESERIALFUNC = my_deserial)", ); + pg_and_generic().verified_stmt( + "CREATE AGGREGATE my_shareable (INT) (SFUNC = my_sfunc, STYPE = internal, FINALFUNC_MODIFY = SHAREABLE, MFINALFUNC = my_mfinal, MSSPACE = 64, MINITCOND = '0')", + ); } #[test] -fn parse_create_aggregate_old_syntax_basetype() { +fn parse_create_aggregate_legacy_syntax() { let stmt = pg_and_generic() .verified_stmt("CREATE AGGREGATE my_avg (BASETYPE = INT, SFUNC = my_sfunc, STYPE = INT)"); match stmt { Statement::CreateAggregate(agg) => { - assert!(agg.args.is_empty()); - assert!(!agg.star_args); + assert_eq!(agg.args, CreateAggregateArgs::Legacy); assert_eq!(agg.options.len(), 3); assert_eq!( agg.options[0], @@ -9665,8 +9686,7 @@ fn parse_create_aggregate_old_syntax_basetype() { .verified_stmt("CREATE AGGREGATE my_avg (SFUNC = my_sfunc, BASETYPE = INT, STYPE = INT)"); match stmt { Statement::CreateAggregate(agg) => { - assert!(agg.args.is_empty()); - assert!(!agg.star_args); + assert_eq!(agg.args, CreateAggregateArgs::Legacy); assert_eq!(agg.options.len(), 3); assert_eq!( agg.options[1], @@ -9675,13 +9695,43 @@ fn parse_create_aggregate_old_syntax_basetype() { } _ => panic!("Expected CreateAggregate, got: {stmt:?}"), } + + // An option whose value cannot start an argument definition: the + // speculative argument-list parse must not leak its error. + pg_and_generic().verified_stmt( + "CREATE AGGREGATE my_min (BASETYPE = INT, SFUNC = my_sfunc, STYPE = INT, SORTOP = <)", + ); + + // Without BASETYPE the statement is still a single-list one, and must not + // gain an empty argument list on the way back out. + pg_and_generic().verified_stmt("CREATE AGGREGATE my_avg (SFUNC = my_sfunc, STYPE = INT)"); } #[test] -fn parse_create_aggregate_unknown_option() { - assert!(pg_and_generic() +fn parse_create_aggregate_rejects_bad_options() { + let expected = "sql parser error: Expected: one of SFUNC or STYPE or SSPACE or FINALFUNC or FINALFUNC_EXTRA or FINALFUNC_MODIFY or COMBINEFUNC or SERIALFUNC or DESERIALFUNC or INITCOND or MSFUNC or MINVFUNC or MSTYPE or MSSPACE or MFINALFUNC or MFINALFUNC_EXTRA or MFINALFUNC_MODIFY or MINITCOND or SORTOP or PARALLEL or HYPOTHETICAL or BASETYPE"; + + let unknown = pg_and_generic() .parse_sql_statements("CREATE AGGREGATE foo (INT) (UNKNOWN_OPTION = bar)") - .is_err()); + .unwrap_err(); + assert_eq!( + unknown.to_string(), + format!("{expected}, found: UNKNOWN_OPTION") + ); + + // A quoted key is an identifier, not the keyword it spells. + let quoted = pg_and_generic() + .parse_sql_statements("CREATE AGGREGATE foo (INT) (\"SFUNC\" = my_sfunc, STYPE = INT)") + .unwrap_err(); + assert_eq!(quoted.to_string(), format!("{expected}, found: \"SFUNC\"")); + + let duplicate = pg_and_generic() + .parse_sql_statements("CREATE AGGREGATE foo (INT) (SFUNC = f, SFUNC = g, STYPE = INT)") + .unwrap_err(); + assert_eq!( + duplicate.to_string(), + "sql parser error: Expected: no duplicate CREATE AGGREGATE option, found: SFUNC" + ); } #[test] From 494520240ec2eaba03e1fa380b40160435377fd8 Mon Sep 17 00:00:00 2001 From: Filipe Guerreiro Date: Fri, 24 Jul 2026 10:39:45 +0900 Subject: [PATCH 14/17] PostgreSQL: pick the CREATE AGGREGATE syntax form by lookahead Speculatively parsing the argument list and falling back on error meant a typo inside a modern argument list was reinterpreted as the legacy form, so `CREATE AGGREGATE foo (INT, ) (SFUNC = f)` pointed at `INT` and demanded an aggregate option. Count the parenthesized lists instead, which is the only thing that distinguishes the two forms, and let each branch report its own error. - name the duplicate option in its error instead of "Expected: no duplicate" - move the option keys next to the match arms they must stay in sync with - stop asserting the whole 22-keyword "one of" list in tests, which breaks on every future option for no signal --- src/ast/ddl.rs | 4 +- src/parser/mod.rs | 115 +++++++++++++++++++++--------------- tests/sqlparser_postgres.rs | 48 +++++++++++---- 3 files changed, 106 insertions(+), 61 deletions(-) diff --git a/src/ast/ddl.rs b/src/ast/ddl.rs index 3522d1e1a8..d8c0e04556 100644 --- a/src/ast/ddl.rs +++ b/src/ast/ddl.rs @@ -6033,8 +6033,8 @@ impl fmt::Display for CreateAggregate { #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] #[cfg_attr(feature = "visitor", derive(Visit, VisitMut))] pub enum CreateAggregateArgs { - /// No argument list at all: the pre-8.2 syntax that instead carries a - /// `BASETYPE` entry in the options list. + /// No argument-list parentheses: the historical form that packs everything, + /// usually including a `BASETYPE` option, into a single list. Legacy, /// The wildcard form `(*)`, used by zero-argument aggregates such as /// `count(*)`. diff --git a/src/parser/mod.rs b/src/parser/mod.rs index c4fa9e07da..feab09ade7 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -7427,56 +7427,26 @@ impl<'a> Parser<'a> { ) -> Result { let name = self.parse_object_name(false)?; - // The keys accepted inside the options list. - const OPTIONS: &[Keyword] = &[ - Keyword::SFUNC, - Keyword::STYPE, - Keyword::SSPACE, - Keyword::FINALFUNC, - Keyword::FINALFUNC_EXTRA, - Keyword::FINALFUNC_MODIFY, - Keyword::COMBINEFUNC, - Keyword::SERIALFUNC, - Keyword::DESERIALFUNC, - Keyword::INITCOND, - Keyword::MSFUNC, - Keyword::MINVFUNC, - Keyword::MSTYPE, - Keyword::MSSPACE, - Keyword::MFINALFUNC, - Keyword::MFINALFUNC_EXTRA, - Keyword::MFINALFUNC_MODIFY, - Keyword::MINITCOND, - Keyword::SORTOP, - Keyword::PARALLEL, - Keyword::HYPOTHETICAL, - Keyword::BASETYPE, - ]; - - // The legacy syntax has a single parenthesized list that carries both - // `BASETYPE` and the remaining options. The modern syntax has a - // separate `(input_arg [, ...])` argument list followed by the options - // list. They are only distinguishable by whether a second parenthesized - // list follows the first, so parse the argument list speculatively and - // fall back to the legacy form when no options list follows it. - let args = match self.maybe_parse(|parser| { - let args = parser.parse_create_aggregate_args()?; - parser.expect_token(&Token::LParen)?; - Ok(args) - })? { - Some(args) => args, - None => { - self.expect_token(&Token::LParen)?; - CreateAggregateArgs::Legacy - } + // The modern syntax is `name (arg [, ...]) (option [, ...])`; the legacy + // one drops the argument list and packs everything, `BASETYPE` included, + // into a single list. Only the number of parenthesized lists tells them + // apart, so look that far ahead before committing to either branch. + let args = if self.peek_second_paren_list() { + self.parse_create_aggregate_args()? + } else { + CreateAggregateArgs::Legacy }; + self.expect_token(&Token::LParen)?; let mut seen: Vec = Vec::new(); let options = self.parse_comma_separated(|parser| { let token = parser.peek_token(); - let keyword = parser.expect_one_of_keywords(OPTIONS)?; + let keyword = parser.parse_create_aggregate_option_key()?; if seen.contains(&keyword) { - return parser.expected("no duplicate CREATE AGGREGATE option", token); + return parser_err!( + format!("Duplicate CREATE AGGREGATE option: {keyword:?}"), + token.span.start + ); } seen.push(keyword); parser.parse_create_aggregate_option(keyword) @@ -7505,6 +7475,29 @@ impl<'a> Parser<'a> { Ok(args) } + /// True when the parenthesized list starting at the current position is + /// followed by a second one. + fn peek_second_paren_list(&self) -> bool { + if self.peek_token_ref().token != Token::LParen { + return false; + } + let mut depth = 0usize; + for offset in 0.. { + match self.peek_nth_token_ref(offset).token { + Token::LParen => depth += 1, + Token::RParen => { + depth -= 1; + if depth == 0 { + return self.peek_nth_token_ref(offset + 1).token == Token::LParen; + } + } + Token::EOF => break, + _ => {} + } + } + false + } + /// Parse `PARALLEL` qualifier value: `{ SAFE | RESTRICTED | UNSAFE }`. fn parse_function_parallel(&mut self) -> Result { match self.expect_one_of_keywords(&[Keyword::SAFE, Keyword::RESTRICTED, Keyword::UNSAFE])? { @@ -7515,6 +7508,35 @@ impl<'a> Parser<'a> { } } + /// Parse the key of a single `CREATE AGGREGATE` option. Every key listed + /// here needs an arm in [`Self::parse_create_aggregate_option`]. + fn parse_create_aggregate_option_key(&mut self) -> Result { + self.expect_one_of_keywords(&[ + Keyword::SFUNC, + Keyword::STYPE, + Keyword::SSPACE, + Keyword::FINALFUNC, + Keyword::FINALFUNC_EXTRA, + Keyword::FINALFUNC_MODIFY, + Keyword::COMBINEFUNC, + Keyword::SERIALFUNC, + Keyword::DESERIALFUNC, + Keyword::INITCOND, + Keyword::MSFUNC, + Keyword::MINVFUNC, + Keyword::MSTYPE, + Keyword::MSSPACE, + Keyword::MFINALFUNC, + Keyword::MFINALFUNC_EXTRA, + Keyword::MFINALFUNC_MODIFY, + Keyword::MINITCOND, + Keyword::SORTOP, + Keyword::PARALLEL, + Keyword::HYPOTHETICAL, + Keyword::BASETYPE, + ]) + } + /// Parse the value of a single `CREATE AGGREGATE` option, given its /// already-consumed key. fn parse_create_aggregate_option( @@ -7522,11 +7544,10 @@ impl<'a> Parser<'a> { keyword: Keyword, ) -> Result { // Every option but the three valueless flags is spelled `KEY = value`. - let is_flag = matches!( + if !matches!( keyword, Keyword::FINALFUNC_EXTRA | Keyword::MFINALFUNC_EXTRA | Keyword::HYPOTHETICAL - ); - if !is_flag { + ) { self.expect_token(&Token::Eq)?; } diff --git a/tests/sqlparser_postgres.rs b/tests/sqlparser_postgres.rs index 48a3bf853f..2bdfaa2235 100644 --- a/tests/sqlparser_postgres.rs +++ b/tests/sqlparser_postgres.rs @@ -9696,8 +9696,7 @@ fn parse_create_aggregate_legacy_syntax() { _ => panic!("Expected CreateAggregate, got: {stmt:?}"), } - // An option whose value cannot start an argument definition: the - // speculative argument-list parse must not leak its error. + // An option whose value cannot start an argument definition. pg_and_generic().verified_stmt( "CREATE AGGREGATE my_min (BASETYPE = INT, SFUNC = my_sfunc, STYPE = INT, SORTOP = <)", ); @@ -9709,28 +9708,53 @@ fn parse_create_aggregate_legacy_syntax() { #[test] fn parse_create_aggregate_rejects_bad_options() { - let expected = "sql parser error: Expected: one of SFUNC or STYPE or SSPACE or FINALFUNC or FINALFUNC_EXTRA or FINALFUNC_MODIFY or COMBINEFUNC or SERIALFUNC or DESERIALFUNC or INITCOND or MSFUNC or MINVFUNC or MSTYPE or MSSPACE or MFINALFUNC or MFINALFUNC_EXTRA or MFINALFUNC_MODIFY or MINITCOND or SORTOP or PARALLEL or HYPOTHETICAL or BASETYPE"; - let unknown = pg_and_generic() .parse_sql_statements("CREATE AGGREGATE foo (INT) (UNKNOWN_OPTION = bar)") - .unwrap_err(); - assert_eq!( - unknown.to_string(), - format!("{expected}, found: UNKNOWN_OPTION") + .unwrap_err() + .to_string(); + assert!( + unknown.ends_with(", found: UNKNOWN_OPTION"), + "unexpected error: {unknown}" ); - // A quoted key is an identifier, not the keyword it spells. + // Option keys are keywords here, as they are in CREATE OPERATOR, so a + // quoted key is an ordinary identifier and does not name an option. let quoted = pg_and_generic() .parse_sql_statements("CREATE AGGREGATE foo (INT) (\"SFUNC\" = my_sfunc, STYPE = INT)") - .unwrap_err(); - assert_eq!(quoted.to_string(), format!("{expected}, found: \"SFUNC\"")); + .unwrap_err() + .to_string(); + assert!( + quoted.ends_with(", found: \"SFUNC\""), + "unexpected error: {quoted}" + ); let duplicate = pg_and_generic() .parse_sql_statements("CREATE AGGREGATE foo (INT) (SFUNC = f, SFUNC = g, STYPE = INT)") .unwrap_err(); assert_eq!( duplicate.to_string(), - "sql parser error: Expected: no duplicate CREATE AGGREGATE option, found: SFUNC" + "sql parser error: Duplicate CREATE AGGREGATE option: SFUNC" + ); +} + +#[test] +fn parse_create_aggregate_reports_argument_list_errors() { + // A malformed argument list must report itself, not be reinterpreted as the + // legacy single-list form and blamed on the options. + let err = pg_and_generic() + .parse_sql_statements("CREATE AGGREGATE foo (INT, ) (SFUNC = f, STYPE = INT)") + .unwrap_err(); + assert_eq!( + err.to_string(), + "sql parser error: Expected: a data type name, found: )" + ); + + let missing = pg_and_generic() + .parse_sql_statements("CREATE AGGREGATE foo )") + .unwrap_err(); + assert_eq!( + missing.to_string(), + "sql parser error: Expected: (, found: )" ); } From b08f2d0df33de62bd5616e80aa6b244663cd0e77 Mon Sep 17 00:00:00 2001 From: Filipe Guerreiro Date: Fri, 24 Jul 2026 10:48:20 +0900 Subject: [PATCH 15/17] PostgreSQL: treat an unclosed CREATE AGGREGATE argument list as modern syntax The lookahead only committed to the modern form when it found a matching close paren, so `CREATE AGGREGATE foo (INT` fell through to the legacy single-list branch and reported `INT` as an unknown aggregate option. An unclosed list is not the legacy form either. Report the missing `)`. - scan the token slice once instead of re-peeking from the current index - cover parenthesized types, which are what the depth counter is for - anchor the option-key error assertions at both ends --- src/parser/mod.rs | 37 +++++++++++++++++++++---------------- tests/sqlparser_postgres.rs | 26 ++++++++++++++++++++++++-- 2 files changed, 45 insertions(+), 18 deletions(-) diff --git a/src/parser/mod.rs b/src/parser/mod.rs index feab09ade7..ae976a81e7 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -7427,10 +7427,9 @@ impl<'a> Parser<'a> { ) -> Result { let name = self.parse_object_name(false)?; - // The modern syntax is `name (arg [, ...]) (option [, ...])`; the legacy - // one drops the argument list and packs everything, `BASETYPE` included, - // into a single list. Only the number of parenthesized lists tells them - // apart, so look that far ahead before committing to either branch. + // The legacy and modern forms (see [`CreateAggregateArgs::Legacy`]) differ + // only in whether a second parenthesized list follows the first, so look + // that far ahead before committing to either branch. let args = if self.peek_second_paren_list() { self.parse_create_aggregate_args()? } else { @@ -7440,12 +7439,12 @@ impl<'a> Parser<'a> { let mut seen: Vec = Vec::new(); let options = self.parse_comma_separated(|parser| { - let token = parser.peek_token(); + let start = parser.peek_token_ref().span.start; let keyword = parser.parse_create_aggregate_option_key()?; if seen.contains(&keyword) { return parser_err!( format!("Duplicate CREATE AGGREGATE option: {keyword:?}"), - token.span.start + start ); } seen.push(keyword); @@ -7476,26 +7475,33 @@ impl<'a> Parser<'a> { } /// True when the parenthesized list starting at the current position is - /// followed by a second one. + /// followed by a second one, or is never closed. fn peek_second_paren_list(&self) -> bool { - if self.peek_token_ref().token != Token::LParen { + let mut tokens = self + .tokens + .iter() + .skip(self.index) + .map(|token| &token.token) + .filter(|token| !matches!(token, Token::Whitespace(_))); + if tokens.next() != Some(&Token::LParen) { return false; } - let mut depth = 0usize; - for offset in 0.. { - match self.peek_nth_token_ref(offset).token { + let mut depth = 1usize; + for token in tokens.by_ref() { + match token { Token::LParen => depth += 1, Token::RParen => { depth -= 1; if depth == 0 { - return self.peek_nth_token_ref(offset + 1).token == Token::LParen; + return tokens.next() == Some(&Token::LParen); } } - Token::EOF => break, _ => {} } } - false + // An unclosed list is not the single-list form either, so report it as + // the missing `)` it is rather than as an unknown option. + true } /// Parse `PARALLEL` qualifier value: `{ SAFE | RESTRICTED | UNSAFE }`. @@ -7508,8 +7514,7 @@ impl<'a> Parser<'a> { } } - /// Parse the key of a single `CREATE AGGREGATE` option. Every key listed - /// here needs an arm in [`Self::parse_create_aggregate_option`]. + /// Every key listed here needs an arm in [`Self::parse_create_aggregate_option`]. fn parse_create_aggregate_option_key(&mut self) -> Result { self.expect_one_of_keywords(&[ Keyword::SFUNC, diff --git a/tests/sqlparser_postgres.rs b/tests/sqlparser_postgres.rs index 2bdfaa2235..fd3c4275c6 100644 --- a/tests/sqlparser_postgres.rs +++ b/tests/sqlparser_postgres.rs @@ -9661,6 +9661,17 @@ fn parse_create_aggregate_additional_options() { pg_and_generic().verified_stmt( "CREATE AGGREGATE my_sum (INT) (SFUNC = my_sfunc, STYPE = internal, COMBINEFUNC = my_combine, SERIALFUNC = my_serial, DESERIALFUNC = my_deserial)", ); + pg_and_generic().verified_stmt( + "CREATE AGGREGATE my_extra (INT) (SFUNC = my_sfunc, STYPE = internal, FINALFUNC = my_final, FINALFUNC_EXTRA)", + ); + + // Parenthesized types must not close the argument list early. + pg_and_generic().verified_stmt( + "CREATE AGGREGATE my_numeric (NUMERIC(10,2)) (SFUNC = my_sfunc, STYPE = NUMERIC(10,2))", + ); + pg_and_generic().verified_stmt( + "CREATE AGGREGATE my_numeric2 (BASETYPE = NUMERIC(10,2), SFUNC = my_sfunc, STYPE = INT)", + ); pg_and_generic().verified_stmt( "CREATE AGGREGATE my_shareable (INT) (SFUNC = my_sfunc, STYPE = internal, FINALFUNC_MODIFY = SHAREABLE, MFINALFUNC = my_mfinal, MSSPACE = 64, MINITCOND = '0')", ); @@ -9713,7 +9724,8 @@ fn parse_create_aggregate_rejects_bad_options() { .unwrap_err() .to_string(); assert!( - unknown.ends_with(", found: UNKNOWN_OPTION"), + unknown.starts_with("sql parser error: Expected: one of SFUNC") + && unknown.ends_with(", found: UNKNOWN_OPTION"), "unexpected error: {unknown}" ); @@ -9724,7 +9736,8 @@ fn parse_create_aggregate_rejects_bad_options() { .unwrap_err() .to_string(); assert!( - quoted.ends_with(", found: \"SFUNC\""), + quoted.starts_with("sql parser error: Expected: one of SFUNC") + && quoted.ends_with(", found: \"SFUNC\""), "unexpected error: {quoted}" ); @@ -9749,6 +9762,15 @@ fn parse_create_aggregate_reports_argument_list_errors() { "sql parser error: Expected: a data type name, found: )" ); + // An unclosed argument list is not the legacy single-list form either. + let unclosed = pg_and_generic() + .parse_sql_statements("CREATE AGGREGATE foo (INT") + .unwrap_err(); + assert_eq!( + unclosed.to_string(), + "sql parser error: Expected: ), found: EOF" + ); + let missing = pg_and_generic() .parse_sql_statements("CREATE AGGREGATE foo )") .unwrap_err(); From 280b7ab02215260a68174830617d0b59dff791bf Mon Sep 17 00:00:00 2001 From: Filipe Guerreiro Date: Fri, 24 Jul 2026 10:55:08 +0900 Subject: [PATCH 16/17] PostgreSQL: report the missing paren on an unclosed CREATE AGGREGATE list Committing to either form on an unclosed list buries the real error under whichever one that branch hits first. Forcing the modern branch turned `CREATE AGGREGATE foo (SORTOP = <, SFUNC = f, STYPE = INT` into "expected an expression"; forcing the legacy branch blamed the options list for `foo (INT`. Neither form applies to a list that never closes, so say that instead of picking one. - rename the lookahead, which no longer answers only "is there a second list" - stop the scan at a statement boundary - move the parenthesized-type case to its own test, and drop the legacy variant of it, which reached the right branch even with the depth counter broken --- src/parser/mod.rs | 23 +++++++++++++---------- tests/sqlparser_postgres.rs | 37 ++++++++++++++++++++++--------------- 2 files changed, 35 insertions(+), 25 deletions(-) diff --git a/src/parser/mod.rs b/src/parser/mod.rs index ae976a81e7..71cbe8a5fd 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -7430,7 +7430,7 @@ impl<'a> Parser<'a> { // The legacy and modern forms (see [`CreateAggregateArgs::Legacy`]) differ // only in whether a second parenthesized list follows the first, so look // that far ahead before committing to either branch. - let args = if self.peek_second_paren_list() { + let args = if self.peek_create_aggregate_arg_list()? { self.parse_create_aggregate_args()? } else { CreateAggregateArgs::Legacy @@ -7474,9 +7474,13 @@ impl<'a> Parser<'a> { Ok(args) } - /// True when the parenthesized list starting at the current position is - /// followed by a second one, or is never closed. - fn peek_second_paren_list(&self) -> bool { + /// True when the list at the current position is the modern form's argument + /// list, meaning a second parenthesized list follows it. + /// + /// Errors when the list is never closed rather than guessing a form, since + /// committing to either one buries the missing `)` under whichever error + /// that branch happens to hit first. + fn peek_create_aggregate_arg_list(&self) -> Result { let mut tokens = self .tokens .iter() @@ -7484,24 +7488,23 @@ impl<'a> Parser<'a> { .map(|token| &token.token) .filter(|token| !matches!(token, Token::Whitespace(_))); if tokens.next() != Some(&Token::LParen) { - return false; + return Ok(false); } let mut depth = 1usize; - for token in tokens.by_ref() { + while let Some(token) = tokens.next() { match token { Token::LParen => depth += 1, Token::RParen => { depth -= 1; if depth == 0 { - return tokens.next() == Some(&Token::LParen); + return Ok(tokens.next() == Some(&Token::LParen)); } } + Token::SemiColon => break, _ => {} } } - // An unclosed list is not the single-list form either, so report it as - // the missing `)` it is rather than as an unknown option. - true + self.expected_ref(")", &EOF_TOKEN) } /// Parse `PARALLEL` qualifier value: `{ SAFE | RESTRICTED | UNSAFE }`. diff --git a/tests/sqlparser_postgres.rs b/tests/sqlparser_postgres.rs index fd3c4275c6..3b5b4168e5 100644 --- a/tests/sqlparser_postgres.rs +++ b/tests/sqlparser_postgres.rs @@ -9664,16 +9664,17 @@ fn parse_create_aggregate_additional_options() { pg_and_generic().verified_stmt( "CREATE AGGREGATE my_extra (INT) (SFUNC = my_sfunc, STYPE = internal, FINALFUNC = my_final, FINALFUNC_EXTRA)", ); - - // Parenthesized types must not close the argument list early. pg_and_generic().verified_stmt( - "CREATE AGGREGATE my_numeric (NUMERIC(10,2)) (SFUNC = my_sfunc, STYPE = NUMERIC(10,2))", - ); - pg_and_generic().verified_stmt( - "CREATE AGGREGATE my_numeric2 (BASETYPE = NUMERIC(10,2), SFUNC = my_sfunc, STYPE = INT)", + "CREATE AGGREGATE my_shareable (INT) (SFUNC = my_sfunc, STYPE = internal, FINALFUNC_MODIFY = SHAREABLE, MFINALFUNC = my_mfinal, MSSPACE = 64, MINITCOND = '0')", ); +} + +#[test] +fn parse_create_aggregate_parenthesized_arg_type() { + // The two forms are told apart by counting parenthesized lists, so a + // parenthesized type must not be mistaken for the end of the argument list. pg_and_generic().verified_stmt( - "CREATE AGGREGATE my_shareable (INT) (SFUNC = my_sfunc, STYPE = internal, FINALFUNC_MODIFY = SHAREABLE, MFINALFUNC = my_mfinal, MSSPACE = 64, MINITCOND = '0')", + "CREATE AGGREGATE my_numeric (NUMERIC(10,2)) (SFUNC = my_sfunc, STYPE = NUMERIC(10,2))", ); } @@ -9762,14 +9763,20 @@ fn parse_create_aggregate_reports_argument_list_errors() { "sql parser error: Expected: a data type name, found: )" ); - // An unclosed argument list is not the legacy single-list form either. - let unclosed = pg_and_generic() - .parse_sql_statements("CREATE AGGREGATE foo (INT") - .unwrap_err(); - assert_eq!( - unclosed.to_string(), - "sql parser error: Expected: ), found: EOF" - ); + // An unclosed list is reported as the missing `)` it is, whichever form it + // was shaping up to be. + for sql in [ + "CREATE AGGREGATE foo (INT", + "CREATE AGGREGATE foo (SORTOP = <, SFUNC = f, STYPE = INT", + "CREATE AGGREGATE foo (BASETYPE = INT, SFUNC = f", + ] { + let unclosed = pg_and_generic().parse_sql_statements(sql).unwrap_err(); + assert_eq!( + unclosed.to_string(), + "sql parser error: Expected: ), found: EOF", + "unexpected error for {sql}" + ); + } let missing = pg_and_generic() .parse_sql_statements("CREATE AGGREGATE foo )") From a42f9b6e7816a51dfc7521ec681e893942bb466a Mon Sep 17 00:00:00 2001 From: Filipe Guerreiro Date: Fri, 24 Jul 2026 11:05:41 +0900 Subject: [PATCH 17/17] PostgreSQL: name the real token when a CREATE AGGREGATE list hits a semicolon The statement-boundary bail shared the end-of-input error, so `CREATE AGGREGATE foo (INT; SELECT 1` reported `found: EOF` for a semicolon at column 26. Keep the spans while scanning so that path can report the token it actually stopped at, and cover it with a test. Also restore the legacy-form parenthesized-type case dropped when that assertion moved to its own test. --- src/parser/mod.rs | 13 +++++++------ tests/sqlparser_postgres.rs | 17 +++++++++++++++-- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/src/parser/mod.rs b/src/parser/mod.rs index 71cbe8a5fd..8bcc1e6557 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -7485,22 +7485,23 @@ impl<'a> Parser<'a> { .tokens .iter() .skip(self.index) - .map(|token| &token.token) - .filter(|token| !matches!(token, Token::Whitespace(_))); - if tokens.next() != Some(&Token::LParen) { + .filter(|token| !matches!(token.token, Token::Whitespace(_))); + if tokens.next().map(|token| &token.token) != Some(&Token::LParen) { return Ok(false); } let mut depth = 1usize; while let Some(token) = tokens.next() { - match token { + match token.token { Token::LParen => depth += 1, Token::RParen => { depth -= 1; if depth == 0 { - return Ok(tokens.next() == Some(&Token::LParen)); + return Ok(tokens.next().map(|token| &token.token) == Some(&Token::LParen)); } } - Token::SemiColon => break, + // The list is still open at the statement boundary, so stop + // before the next statement's parentheses answer for this one. + Token::SemiColon => return self.expected_ref(")", token), _ => {} } } diff --git a/tests/sqlparser_postgres.rs b/tests/sqlparser_postgres.rs index 3b5b4168e5..61958312b9 100644 --- a/tests/sqlparser_postgres.rs +++ b/tests/sqlparser_postgres.rs @@ -9676,6 +9676,9 @@ fn parse_create_aggregate_parenthesized_arg_type() { pg_and_generic().verified_stmt( "CREATE AGGREGATE my_numeric (NUMERIC(10,2)) (SFUNC = my_sfunc, STYPE = NUMERIC(10,2))", ); + pg_and_generic().verified_stmt( + "CREATE AGGREGATE my_numeric2 (BASETYPE = NUMERIC(10,2), SFUNC = my_sfunc, STYPE = INT)", + ); } #[test] @@ -9763,8 +9766,8 @@ fn parse_create_aggregate_reports_argument_list_errors() { "sql parser error: Expected: a data type name, found: )" ); - // An unclosed list is reported as the missing `)` it is, whichever form it - // was shaping up to be. + // Every unclosed-list shape reports the same missing `)`, rather than the + // form-specific error a guessed branch would hit first. for sql in [ "CREATE AGGREGATE foo (INT", "CREATE AGGREGATE foo (SORTOP = <, SFUNC = f, STYPE = INT", @@ -9778,6 +9781,16 @@ fn parse_create_aggregate_reports_argument_list_errors() { ); } + // A statement boundary ends the search, so the following statement's + // parentheses cannot answer for this one. + let semicolon = pg_and_generic() + .parse_sql_statements("CREATE AGGREGATE foo (INT; SELECT (1) (2)") + .unwrap_err(); + assert_eq!( + semicolon.to_string(), + "sql parser error: Expected: ), found: ;" + ); + let missing = pg_and_generic() .parse_sql_statements("CREATE AGGREGATE foo )") .unwrap_err();