LucaCappelletti94 commented on code in PR #2316:
URL:
https://github.com/apache/datafusion-sqlparser-rs/pull/2316#discussion_r4111415330
##########
src/parser/mod.rs:
##########
@@ -7499,6 +7492,226 @@ 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_separated(Parser::parse_function_arg)?)
+ };
Review Comment:
You should accept ordered-set argument lists. `pg_dump` writes every
ordered-set aggregate as `CREATE AGGREGATE public.my_pct(double precision ORDER
BY double precision) (...)`, and PostgreSQL 18 rejects `HYPOTHETICAL` anywhere
else (`only ordered-set aggregates can be hypothetical`), yet `(FLOAT8 ORDER BY
FLOAT8)`, `(ORDER BY anyelement)` and `(VARIADIC "any" ORDER BY VARIADIC
"any")` all fail to parse here.
Could look something like this, I will try to prepare some red tests shortly.
```suggestion
} else {
let direct = if self.peek_keyword(Keyword::ORDER) {
vec![]
} else {
self.parse_comma_separated(Parser::parse_function_arg)?
};
if self.parse_keywords(&[Keyword::ORDER, Keyword::BY]) {
CreateAggregateArgs::OrderedSet {
direct,
aggregated:
self.parse_comma_separated(Parser::parse_function_arg)?,
}
} else {
CreateAggregateArgs::List(direct)
}
};
```
##########
src/ast/spans.rs:
##########
@@ -2532,6 +2533,65 @@ impl Spanned for AlterTable {
}
}
+impl Spanned for CreateAggregate {
+ fn span(&self) -> Span {
+ union_spans(
+ core::iter::once(self.name.span())
+ .chain(core::iter::once(self.args.span()))
+ .chain(self.options.iter().map(|option| option.span())),
+ )
+ }
+}
+
+impl Spanned for CreateAggregateArgs {
+ fn span(&self) -> Span {
+ match self {
+ CreateAggregateArgs::Legacy | CreateAggregateArgs::Star =>
Span::empty(),
+ CreateAggregateArgs::List(args) =>
union_spans(args.iter().map(|arg| arg.span())),
Review Comment:
```suggestion
CreateAggregateArgs::List(args) =>
union_spans(args.iter().map(|arg| arg.span())),
CreateAggregateArgs::OrderedSet { direct, aggregated } => {
union_spans(direct.iter().chain(aggregated).map(|arg|
arg.span()))
}
```
##########
src/ast/ddl.rs:
##########
@@ -6078,3 +6078,168 @@ 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: `(NUMERIC)`,
+ /// `(input INT, VARIADIC tail TEXT)`.
+ List(Vec<OperateFunctionArg>),
Review Comment:
Adding an ordered-set variant I proposed in the other suggestions. I am
unsure whether it could be representable in some other manner.
```suggestion
List(Vec<OperateFunctionArg>),
/// An ordered-set argument list: `(FLOAT8 ORDER BY FLOAT8)`,
/// `(ORDER BY anyelement)`.
OrderedSet {
/// The direct arguments before `ORDER BY`, possibly empty.
direct: Vec<OperateFunctionArg>,
/// The aggregated arguments after `ORDER BY`.
aggregated: Vec<OperateFunctionArg>,
},
```
##########
tests/sqlparser_postgres.rs:
##########
@@ -9652,6 +9652,279 @@ 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_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: )"
+ );
+}
+
+#[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)",
+ );
Review Comment:
Adding red test and moving `HYPOTHETICAL` onto the hypothetical-set one.
```suggestion
pg_and_generic().verified_stmt(
"CREATE AGGREGATE percentile (FLOAT8 ORDER BY FLOAT8) (SFUNC =
ordered_set_transition, STYPE = internal, FINALFUNC = percentile_final,
FINALFUNC_MODIFY = READ_WRITE)",
);
pg_and_generic().verified_stmt(
"CREATE AGGREGATE my_mode (ORDER BY anyelement) (SFUNC =
ordered_set_transition, STYPE = internal, FINALFUNC = mode_final,
FINALFUNC_EXTRA)",
);
pg_and_generic().verified_stmt(
"CREATE AGGREGATE my_rank (VARIADIC \"any\" ORDER BY VARIADIC
\"any\") (SFUNC = ordered_set_transition_multi, STYPE = internal, FINALFUNC =
rank_final, FINALFUNC_EXTRA, HYPOTHETICAL)",
);
```
##########
tests/sqlparser_postgres.rs:
##########
@@ -9652,6 +9652,279 @@ 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_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: )"
+ );
+}
+
+#[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
= OPERATOR(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)",
+ );
+ pg_and_generic().verified_stmt(
+ "CREATE AGGREGATE my_extra (INT) (SFUNC = my_sfunc, STYPE = internal,
FINALFUNC = my_final, FINALFUNC_EXTRA)",
+ );
+ 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_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_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]
+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_eq!(agg.args, CreateAggregateArgs::Legacy);
+ 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_eq!(agg.args, CreateAggregateArgs::Legacy);
+ assert_eq!(agg.options.len(), 3);
+ assert_eq!(
+ agg.options[1],
+ CreateAggregateOption::BaseType(DataType::Int(None))
+ );
+ }
+ _ => panic!("Expected CreateAggregate, got: {stmt:?}"),
+ }
+
+ // 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 = <)",
+ );
+
+ // 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_rejects_bad_options() {
+ let unknown = pg_and_generic()
+ .parse_sql_statements("CREATE AGGREGATE foo (INT) (UNKNOWN_OPTION =
bar)")
+ .unwrap_err()
+ .to_string();
+ assert!(
+ unknown.starts_with("sql parser error: Expected: one of SFUNC")
+ && unknown.ends_with(", found: UNKNOWN_OPTION"),
+ "unexpected error: {unknown}"
+ );
+
+ // 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()
+ .to_string();
+ assert!(
+ quoted.starts_with("sql parser error: Expected: one of SFUNC")
+ && 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: Duplicate CREATE AGGREGATE option: SFUNC"
+ );
Review Comment:
You should remove the duplicate-option assertion together with the check.
```suggestion
```
##########
src/parser/mod.rs:
##########
@@ -7499,6 +7492,226 @@ 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)
+ })?;
Review Comment:
You should drop the duplicate-option check. PostgreSQL 18 accepts `CREATE
AGGREGATE dup(int4) (SFUNC = int4mi, STYPE = int4, SFUNC = int4pl)` and keeps
the last value (`aggtransfn` is `int4pl`), so this rejects valid SQL.
```suggestion
let options = self.parse_comma_separated(|parser| {
let keyword = parser.parse_create_aggregate_option_key()?;
parser.parse_create_aggregate_option(keyword)
})?;
```
##########
src/ast/ddl.rs:
##########
@@ -6078,3 +6078,168 @@ 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))?,
Review Comment:
And, following the above refactoring, also the display is needed:
```suggestion
CreateAggregateArgs::List(args) => write!(f, " ({})",
display_comma_separated(args))?,
CreateAggregateArgs::OrderedSet { direct, aggregated } => {
write!(f, " (")?;
if !direct.is_empty() {
write!(f, "{} ", display_comma_separated(direct))?;
}
write!(f, "ORDER BY {})",
display_comma_separated(aggregated))?;
}
```
##########
src/parser/mod.rs:
##########
Review Comment:
You should stop the name/type disambiguation at `ORDER`, since it currently
reads `FLOAT8 ORDER` as an argument named `FLOAT8` of type `ORDER`. `ORDER` is
reserved in PostgreSQL, so no valid `CREATE FUNCTION` argument changes.
```suggestion
// DEFAULT and ORDER will be parsed as `DataType::Custom`, which is
undesirable in this context
fn parse_data_type_no_default(parser: &mut Parser) ->
Result<DataType, ParserError> {
if parser.peek_keyword(Keyword::DEFAULT) ||
parser.peek_keyword(Keyword::ORDER) {
```
--
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]
