LucaCappelletti94 commented on code in PR #2316:
URL: 
https://github.com/apache/datafusion-sqlparser-rs/pull/2316#discussion_r4108558184


##########
src/ast/ddl.rs:
##########
@@ -6078,3 +6078,165 @@ impl Spanned for CreateForeignTable {
         )
     }
 }
+
+/// CREATE AGGREGATE statement.
+/// See <https://www.postgresql.org/docs/current/sql-createaggregate.html>
+#[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,
+    /// 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<CreateAggregateOption>,
+}
+
+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)?;
+        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 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(*)`.
+    Star,
+    /// An explicit argument list, possibly empty: `()`, `(NUMERIC)`,
+    /// `(input INT, VARIADIC tail TEXT)`.
+    List(Vec<OperateFunctionArg>),
+}
+
+impl From<CreateAggregate> for crate::ast::Statement {
+    fn from(v: CreateAggregate) -> Self {
+        crate::ast::Statement::CreateAggregate(v)
+    }
+}
+
+/// A single option in a `CREATE AGGREGATE` options list.
+///
+/// See <https://www.postgresql.org/docs/current/sql-createaggregate.html>
+#[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`
+    StateTransitionFunction(ObjectName),
+    /// `STYPE = state_data_type`
+    StateDataType(DataType),
+    /// `SSPACE = state_data_size` (in bytes)
+    StateDataSize(u64),
+    /// `FINALFUNC = final_function`
+    FinalFunction(ObjectName),
+    /// `FINALFUNC_EXTRA`. Passes extra dummy arguments to the final function.
+    FinalFunctionExtra,
+    /// `FINALFUNC_MODIFY = { READ_ONLY | SHAREABLE | READ_WRITE }`
+    FinalFunctionModify(AggregateModifyKind),
+    /// `COMBINEFUNC = combine_function`
+    CombineFunction(ObjectName),
+    /// `SERIALFUNC = serial_function`
+    SerialFunction(ObjectName),
+    /// `DESERIALFUNC = deserial_function`
+    DeserialFunction(ObjectName),
+    /// `INITCOND = initial_condition` (a string literal)
+    InitialCondition(ValueWithSpan),
+    /// `MSFUNC = moving_state_transition_function`
+    MovingStateTransitionFunction(ObjectName),
+    /// `MINVFUNC = moving_inverse_transition_function`
+    MovingInverseTransitionFunction(ObjectName),
+    /// `MSTYPE = moving_state_data_type`
+    MovingStateDataType(DataType),
+    /// `MSSPACE = moving_state_data_size` (in bytes)
+    MovingStateDataSize(u64),
+    /// `MFINALFUNC = moving_final_function`
+    MovingFinalFunction(ObjectName),
+    /// `MFINALFUNC_EXTRA`
+    MovingFinalFunctionExtra,
+    /// `MFINALFUNC_MODIFY = { READ_ONLY | SHAREABLE | READ_WRITE }`
+    MovingFinalFunctionModify(AggregateModifyKind),
+    /// `MINITCOND = moving_initial_condition` (a string literal)
+    MovingInitialCondition(ValueWithSpan),
+    /// `SORTOP = sort_operator`
+    SortOperator(ObjectName),
+    /// `PARALLEL = { SAFE | RESTRICTED | UNSAFE }`
+    Parallel(FunctionParallel),
+    /// `HYPOTHETICAL`. Marks the aggregate as hypothetical-set.
+    Hypothetical,
+    /// `BASETYPE = base_type` (old aggregate syntax).
+    BaseType(DataType),
+}
+
+impl fmt::Display for CreateAggregateOption {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match self {
+            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}"),

Review Comment:
   You should render a qualified sort operator inside `OPERATOR(...)`, as 
`ORDER BY ... USING` does in `query.rs`. PostgreSQL rejects the bare `SORTOP = 
pg_catalog.<` with `syntax error at or near "<"`.
   
   ```suggestion
               Self::SortOperator(name) if name.0.len() > 1 => write!(f, 
"SORTOP = OPERATOR({name})"),
               Self::SortOperator(name) => write!(f, "SORTOP = {name}"),
   ```



##########
src/parser/mod.rs:
##########
@@ -7499,6 +7492,218 @@ 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<CreateAggregate, ParserError> {
+        let name = self.parse_object_name(false)?;
+
+        // 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_create_aggregate_arg_list()? {
+            self.parse_create_aggregate_args()?
+        } else {
+            CreateAggregateArgs::Legacy
+        };
+        self.expect_token(&Token::LParen)?;
+
+        let mut seen: Vec<Keyword> = Vec::new();
+        let options = self.parse_comma_separated(|parser| {
+            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:?}"),
+                    start
+                );
+            }
+            seen.push(keyword);
+            parser.parse_create_aggregate_option(keyword)
+        })?;
+        self.expect_token(&Token::RParen)?;
+
+        Ok(CreateAggregate {
+            or_replace,
+            name,
+            args,
+            options,
+        })
+    }
+
+    /// Parse the argument list of a `CREATE AGGREGATE`: `(*)` or `(arg [, 
...])`.
+    fn parse_create_aggregate_args(&mut self) -> Result<CreateAggregateArgs, 
ParserError> {
+        self.expect_token(&Token::LParen)?;
+        let args = if self.consume_token(&Token::Mul) {
+            CreateAggregateArgs::Star
+        } else {
+            CreateAggregateArgs::List(
+                self.parse_comma_separated0(Parser::parse_function_arg, 
Token::RParen)?,
+            )
+        };
+        self.expect_token(&Token::RParen)?;
+        Ok(args)
+    }
+
+    /// 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<bool, ParserError> {
+        let mut tokens = self
+            .tokens
+            .iter()
+            .skip(self.index)
+            .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.token {
+                Token::LParen => depth += 1,
+                Token::RParen => {
+                    depth -= 1;
+                    if depth == 0 {
+                        return Ok(tokens.next().map(|token| &token.token) == 
Some(&Token::LParen));
+                    }
+                }
+                // 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),
+                _ => {}
+            }
+        }
+        self.expected_ref(")", &EOF_TOKEN)
+    }
+
+    /// Parse `PARALLEL` qualifier value: `{ SAFE | RESTRICTED | UNSAFE }`.
+    fn parse_function_parallel(&mut self) -> Result<FunctionParallel, 
ParserError> {
+        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()),
+        }
+    }
+
+    /// Every key listed here needs an arm in 
[`Self::parse_create_aggregate_option`].
+    fn parse_create_aggregate_option_key(&mut self) -> Result<Keyword, 
ParserError> {
+        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(
+        &mut self,
+        keyword: Keyword,
+    ) -> Result<CreateAggregateOption, ParserError> {
+        // Every option but the three valueless flags is spelled `KEY = value`.
+        if !matches!(
+            keyword,
+            Keyword::FINALFUNC_EXTRA | Keyword::MFINALFUNC_EXTRA | 
Keyword::HYPOTHETICAL
+        ) {
+            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)?)
+            }
+            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)?)
+            }
+            Keyword::FINALFUNC_MODIFY => {
+                
CreateAggregateOption::FinalFunctionModify(self.parse_aggregate_modify_kind()?)
+            }
+            Keyword::COMBINEFUNC => {
+                
CreateAggregateOption::CombineFunction(self.parse_object_name(false)?)
+            }
+            Keyword::SERIALFUNC => {
+                
CreateAggregateOption::SerialFunction(self.parse_object_name(false)?)
+            }
+            Keyword::DESERIALFUNC => {
+                
CreateAggregateOption::DeserialFunction(self.parse_object_name(false)?)
+            }
+            Keyword::INITCOND => 
CreateAggregateOption::InitialCondition(self.parse_value()?),
+            Keyword::MSFUNC => {
+                
CreateAggregateOption::MovingStateTransitionFunction(self.parse_object_name(false)?)
+            }
+            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()?)
+            }
+            Keyword::MFINALFUNC => {
+                
CreateAggregateOption::MovingFinalFunction(self.parse_object_name(false)?)
+            }
+            Keyword::MFINALFUNC_MODIFY => 
CreateAggregateOption::MovingFinalFunctionModify(
+                self.parse_aggregate_modify_kind()?,
+            ),
+            Keyword::MINITCOND => {
+                
CreateAggregateOption::MovingInitialCondition(self.parse_value()?)
+            }
+            Keyword::SORTOP => 
CreateAggregateOption::SortOperator(self.parse_operator_name()?),

Review Comment:
   You should accept the `OPERATOR(schema.op)` form here, as `COMMUTATOR` 
already does in `CREATE OPERATOR`. `pg_dump` writes every sort operator that 
way (`SORTOP = OPERATOR(pg_catalog.<)`), so dumped aggregates currently fail 
with `Expected: ), found: (`.
   
   ```suggestion
               Keyword::SORTOP => {
                   let operator = if self.parse_keyword(Keyword::OPERATOR) {
                       self.expect_token(&Token::LParen)?;
                       let operator = self.parse_operator_name()?;
                       self.expect_token(&Token::RParen)?;
                       operator
                   } else {
                       self.parse_operator_name()?
                   };
                   CreateAggregateOption::SortOperator(operator)
               }
   ```



##########
tests/sqlparser_postgres.rs:
##########
@@ -9652,6 +9652,280 @@ 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<String> {
+    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')";
+    let stmt = pg_and_generic().verified_stmt(sql);
+    match stmt {
+        Statement::CreateAggregate(agg) => {
+            assert!(!agg.or_replace);
+            assert_eq!(agg.name.to_string(), "myavg");
+            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");
+            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_span() {
+    let sql = "CREATE AGGREGATE myavg (NUMERIC) (SFUNC = numeric_avg_accum, 
STYPE = internal, FINALFUNC = numeric_avg, INITCOND = '0')";
+    let mut parser = Parser::new(&PostgreSqlDialect {})
+        .try_with_sql(sql)
+        .unwrap();
+    // From the aggregate name through the last spanned option value.
+    assert_eq!(
+        parser.parse_statement().unwrap().span(),
+        Span::new(Location::new(1, 18), Location::new(1, 119))
+    );
+}
+
+#[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_and_generic().verified_stmt(sql);
+    match stmt {
+        Statement::CreateAggregate(agg) => {
+            assert!(agg.or_replace);
+            assert_eq!(agg.name.to_string(), "sum2");
+            assert_eq!(aggregate_args(&agg.args), ["INT4", "INT4"]);
+            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_and_generic().verified_stmt(sql);
+    match stmt {
+        Statement::CreateAggregate(agg) => {
+            assert!(!agg.or_replace);
+            assert_eq!(agg.name.to_string(), "moving_sum");
+            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");
+            assert_eq!(agg.options[6].to_string(), "MFINALFUNC_MODIFY = 
READ_ONLY");
+        }
+        _ => 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_eq!(agg.args, CreateAggregateArgs::Star);
+            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_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 =
+        "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!(
+                aggregate_args(&agg.args),
+                ["input INT", "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.<)",

Review Comment:
   You should round-trip the form PostgreSQL accepts and `pg_dump` emits. The 
current string is a syntax error in PostgreSQL 18, and this one fails on the 
current parser.
   
   ```suggestion
           "CREATE AGGREGATE my_min2 (INT) (SFUNC = my_sfunc, STYPE = INT, 
SORTOP = OPERATOR(pg_catalog.<))",
   ```



##########
src/parser/mod.rs:
##########
@@ -7499,6 +7492,218 @@ 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<CreateAggregate, ParserError> {
+        let name = self.parse_object_name(false)?;
+
+        // 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_create_aggregate_arg_list()? {
+            self.parse_create_aggregate_args()?
+        } else {
+            CreateAggregateArgs::Legacy
+        };
+        self.expect_token(&Token::LParen)?;
+
+        let mut seen: Vec<Keyword> = Vec::new();
+        let options = self.parse_comma_separated(|parser| {
+            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:?}"),
+                    start
+                );
+            }
+            seen.push(keyword);
+            parser.parse_create_aggregate_option(keyword)
+        })?;
+        self.expect_token(&Token::RParen)?;
+
+        Ok(CreateAggregate {
+            or_replace,
+            name,
+            args,
+            options,
+        })
+    }
+
+    /// Parse the argument list of a `CREATE AGGREGATE`: `(*)` or `(arg [, 
...])`.
+    fn parse_create_aggregate_args(&mut self) -> Result<CreateAggregateArgs, 
ParserError> {
+        self.expect_token(&Token::LParen)?;
+        let args = if self.consume_token(&Token::Mul) {
+            CreateAggregateArgs::Star
+        } else {
+            CreateAggregateArgs::List(
+                self.parse_comma_separated0(Parser::parse_function_arg, 
Token::RParen)?,
+            )

Review Comment:
   You should require at least one argument. PostgreSQL rejects `CREATE 
AGGREGATE my_agg () (...)` with `syntax error at or near ")"`, and a 
zero-argument aggregate is spelled `(*)`, which `Star` already covers.
   
   ```suggestion            
CreateAggregateArgs::List(self.parse_comma_separated(Parser::parse_function_arg)?)
   ```



##########
tests/sqlparser_postgres.rs:
##########
@@ -9652,6 +9652,280 @@ 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<String> {
+    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')";
+    let stmt = pg_and_generic().verified_stmt(sql);
+    match stmt {
+        Statement::CreateAggregate(agg) => {
+            assert!(!agg.or_replace);
+            assert_eq!(agg.name.to_string(), "myavg");
+            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");
+            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_span() {
+    let sql = "CREATE AGGREGATE myavg (NUMERIC) (SFUNC = numeric_avg_accum, 
STYPE = internal, FINALFUNC = numeric_avg, INITCOND = '0')";
+    let mut parser = Parser::new(&PostgreSqlDialect {})
+        .try_with_sql(sql)
+        .unwrap();
+    // From the aggregate name through the last spanned option value.
+    assert_eq!(
+        parser.parse_statement().unwrap().span(),
+        Span::new(Location::new(1, 18), Location::new(1, 119))
+    );
+}
+
+#[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_and_generic().verified_stmt(sql);
+    match stmt {
+        Statement::CreateAggregate(agg) => {
+            assert!(agg.or_replace);
+            assert_eq!(agg.name.to_string(), "sum2");
+            assert_eq!(aggregate_args(&agg.args), ["INT4", "INT4"]);
+            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_and_generic().verified_stmt(sql);
+    match stmt {
+        Statement::CreateAggregate(agg) => {
+            assert!(!agg.or_replace);
+            assert_eq!(agg.name.to_string(), "moving_sum");
+            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");
+            assert_eq!(agg.options[6].to_string(), "MFINALFUNC_MODIFY = 
READ_ONLY");
+        }
+        _ => 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_eq!(agg.args, CreateAggregateArgs::Star);
+            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_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:?}"),
+    }

Review Comment:
   You should turn this test into the negative case. It fails on the current 
parser because `()` parses.
   
   ```suggestion
   fn parse_create_aggregate_rejects_empty_args() {
       assert_eq!(
           pg_and_generic()
               .parse_sql_statements("CREATE AGGREGATE my_agg () (SFUNC = 
my_sfunc, STYPE = INT)")
               .unwrap_err()
               .to_string(),
           "sql parser error: Expected: a data type name, found: )"
       );
   ```



##########
src/ast/ddl.rs:
##########
@@ -6078,3 +6078,165 @@ impl Spanned for CreateForeignTable {
         )
     }
 }
+
+/// CREATE AGGREGATE statement.
+/// See <https://www.postgresql.org/docs/current/sql-createaggregate.html>
+#[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,
+    /// 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<CreateAggregateOption>,
+}
+
+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)?;
+        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 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(*)`.
+    Star,
+    /// An explicit argument list, possibly empty: `()`, `(NUMERIC)`,
+    /// `(input INT, VARIADIC tail TEXT)`.

Review Comment:
   You should drop `()` from the documented forms, since PostgreSQL does not 
accept it.
   
   ```suggestion
       /// An explicit argument list: `(NUMERIC)`, `(input INT, VARIADIC tail 
TEXT)`.
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to