You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "fmguerreiro (via GitHub)" <gi...@apache.org> on 2026/04/18 09:08:51 UTC
[PR] PostgreSQL: add support for CREATE AGGREGATE [datafusion-sqlparser-rs]
fmguerreiro opened a new pull request, #2316:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2316
## Summary
Adds PostgreSQL grammar for \`CREATE [OR REPLACE] AGGREGATE\`, including the full option set from the PostgreSQL spec (https://www.postgresql.org/docs/current/sql-createaggregate.html).
## What this supports
- \`CREATE AGGREGATE name (arg_type [, ...]) (options)\`
- \`CREATE OR REPLACE AGGREGATE\`
- Aggregate options: \`SFUNC\`, \`STYPE\`, \`SSPACE\`, \`FINALFUNC\`, \`FINALFUNC_EXTRA\`, \`FINALFUNC_MODIFY\`, \`COMBINEFUNC\`, \`SERIALFUNC\`, \`DESERIALFUNC\`, \`INITCOND\`, \`MSFUNC\`, \`MINVFUNC\`, \`MSTYPE\`, \`MSSPACE\`, \`MFINALFUNC\`, \`MFINALFUNC_EXTRA\`, \`MFINALFUNC_MODIFY\`, \`MINITCOND\`, \`SORTOP\`, \`PARALLEL\`, \`HYPOTHETICAL\`
- \`FINALFUNC_MODIFY\` / \`MFINALFUNC_MODIFY\` kinds: \`READ_ONLY\`, \`SHAREABLE\`, \`READ_WRITE\`
## Changes
- \`CreateAggregate\` struct plus \`CreateAggregateOption\` and \`AggregateModifyKind\` enums in \`src/ast/ddl.rs\` with \`fmt::Display\` + \`From<_> for Statement\`.
- New \`Statement::CreateAggregate\` variant and re-exports.
- \`Spanned\` arm (\`Span::empty()\` stub — happy to extend with a real span if reviewers prefer).
- \`parse_create_aggregate\` in \`src/parser/mod.rs\`.
- Three round-trip tests covering the basic form, \`OR REPLACE\` + \`PARALLEL\`, and moving-aggregate options.
## Notes
The final commit (\`fix(spans): add Span::empty() arm for CreateAggregate\`) is included in the series; if reviewers prefer a real span implementation before merge, happy to replace the stub.
Originally shipped via the \`pgmold-sqlparser\` fork; tested against real PostgreSQL 13–17 schemas in [pgmold](https://github.com/fmguerreiro/pgmold).
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] PostgreSQL: add support for CREATE AGGREGATE [datafusion-sqlparser-rs]
Posted by "LucaCappelletti94 (via GitHub)" <gi...@apache.org>.
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] PostgreSQL: add support for CREATE AGGREGATE [datafusion-sqlparser-rs]
Posted by "LucaCappelletti94 (via GitHub)" <gi...@apache.org>.
LucaCappelletti94 merged PR #2316:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2316
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] PostgreSQL: add support for CREATE AGGREGATE [datafusion-sqlparser-rs]
Posted by "LucaCappelletti94 (via GitHub)" <gi...@apache.org>.
LucaCappelletti94 commented on PR #2316:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2316#issuecomment-6234370925
Make sure CI is green please, after I'll review again.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] PostgreSQL: add support for CREATE AGGREGATE [datafusion-sqlparser-rs]
Posted by "LucaCappelletti94 (via GitHub)" <gi...@apache.org>.
LucaCappelletti94 commented on code in PR #2316:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2316#discussion_r4090652314
##########
src/ast/spans.rs:
##########
@@ -2528,6 +2529,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())),
+ }
+ }
+}
+
+impl Spanned for OperateFunctionArg {
+ fn span(&self) -> Span {
+ union_spans(
+ self.name
+ .iter()
+ .map(|name| name.span)
+ .chain(self.default_expr.iter().map(|expr| expr.span())),
+ )
+ }
+}
+
+impl Spanned for CreateAggregateOption {
+ fn span(&self) -> Span {
+ match self {
+ CreateAggregateOption::StateTransitionFunction(name)
+ | CreateAggregateOption::FinalFunction(name)
Review Comment:
I am unsure whether these are covered (and llvm does not instrument the lines) or only a subset of these are tested
##########
src/parser/mod.rs:
##########
@@ -5317,13 +5317,15 @@ 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 self.parse_keyword(Keyword::SCHEMA) {
self.parse_create_schema(or_replace)
} else if self.parse_keyword(Keyword::WAREHOUSE) {
self.parse_create_warehouse(or_replace).map(Into::into)
} else if or_replace {
self.expected_ref(
- "[EXTERNAL] TABLE or [MATERIALIZED] VIEW or FUNCTION or SCHEMA or WAREHOUSE after CREATE OR REPLACE",
+ "[EXTERNAL] TABLE or [MATERIALIZED] VIEW or FUNCTION or AGGREGATE or SCHEMA or WAREHOUSE after CREATE OR REPLACE",
Review Comment:
Please add a test to cover this case
##########
src/parser/mod.rs:
##########
@@ -7460,6 +7453,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()?),
+ 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());
+ }
+ })
+ }
+
+ /// Parse the value of `FINALFUNC_MODIFY` / `MFINALFUNC_MODIFY`.
+ fn parse_aggregate_modify_kind(&mut self) -> Result<AggregateModifyKind, ParserError> {
+ 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(
Review Comment:
Also this appears not to be covered by tests
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] PostgreSQL: add support for CREATE AGGREGATE [datafusion-sqlparser-rs]
Posted by "fmguerreiro (via GitHub)" <gi...@apache.org>.
fmguerreiro commented on code in PR #2316:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2316#discussion_r4021783510
##########
src/ast/mod.rs:
##########
@@ -10253,16 +10260,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())
+ }
+}
+
Review Comment:
the aggregate option prints `PARALLEL = SAFE` while the function form prints `PARALLEL SAFE`, so as_str gives the bare keyword without duplicating the strings.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] PostgreSQL: add support for CREATE AGGREGATE [datafusion-sqlparser-rs]
Posted by "fmguerreiro (via GitHub)" <gi...@apache.org>.
fmguerreiro commented on code in PR #2316:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2316#discussion_r4021785532
##########
src/ast/spans.rs:
##########
@@ -2518,6 +2519,12 @@ impl Spanned for AlterTable {
}
}
+impl Spanned for CreateAggregate {
+ fn span(&self) -> Span {
+ Span::empty()
+ }
+}
+
Review Comment:
fixed in c0d2a577.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] PostgreSQL: add support for CREATE AGGREGATE [datafusion-sqlparser-rs]
Posted by "fmguerreiro (via GitHub)" <gi...@apache.org>.
fmguerreiro commented on PR #2316:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2316#issuecomment-6262875602
> Make sure CI is green please, after I'll review again.
sorry about that, green now, whenever you get the chance!
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] PostgreSQL: add support for CREATE AGGREGATE [datafusion-sqlparser-rs]
Posted by "fmguerreiro (via GitHub)" <gi...@apache.org>.
fmguerreiro commented on code in PR #2316:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2316#discussion_r4114326805
##########
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:
fixed in 41ef183d.
##########
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:
fixed in 41ef183d.
##########
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:
fixed in 41ef183d.
##########
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:
fixed in 41ef183d. `()` is a parse error now.
##########
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:
fixed in 41ef183d.
##########
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:
done in 41ef183d, asserts the error your suggestion had.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] PostgreSQL: add support for CREATE AGGREGATE [datafusion-sqlparser-rs]
Posted by "fmguerreiro (via GitHub)" <gi...@apache.org>.
fmguerreiro commented on code in PR #2316:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2316#discussion_r4333891187
##########
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:
fixed in ea2bfe88.
##########
src/parser/mod.rs:
##########
Review Comment:
fixed in ea2bfe88.
##########
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:
fixed in ea2bfe88.
##########
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:
fixed in ea2bfe88.
##########
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:
fixed in ea2bfe88.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] PostgreSQL: add support for CREATE AGGREGATE [datafusion-sqlparser-rs]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #2316:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2316#issuecomment-5803420314
## [Codecov](https://app.codecov.io/gh/apache/datafusion-sqlparser-rs/pull/2316?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `79.47598%` with `47 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 80.95%. Comparing base ([`9ae00e7`](https://app.codecov.io/gh/apache/datafusion-sqlparser-rs/commit/9ae00e73b54ebbca2341c3de22312a8011e98757?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`a95f12c`](https://app.codecov.io/gh/apache/datafusion-sqlparser-rs/commit/a95f12c0ea49a935ed64efb8abc30dcbcb91d836?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/datafusion-sqlparser-rs/pull/2316?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [src/parser/mod.rs](https://app.codecov.io/gh/apache/datafusion-sqlparser-rs/pull/2316?src=pr&el=tree&filepath=src%2Fparser%2Fmod.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL3BhcnNlci9tb2QucnM=) | 77.30% | [6 Missing and 26 partials :warning: ](https://app.codecov.io/gh/apache/datafusion-sqlparser-rs/pull/2316?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [src/ast/spans.rs](https://app.codecov.io/gh/apache/datafusion-sqlparser-rs/pull/2316?src=pr&el=tree&filepath=src%2Fast%2Fspans.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL2FzdC9zcGFucy5ycw==) | 70.58% | [7 Missing and 3 partials :warning: ](https://app.codecov.io/gh/apache/datafusion-sqlparser-rs/pull/2316?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [src/ast/ddl.rs](https://app.codecov.io/gh/apache/datafusion-sqlparser-rs/pull/2316?src=pr&el=tree&filepath=src%2Fast%2Fddl.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL2FzdC9kZGwucnM=) | 89.13% | [0 Missing and 5 partials :warning: ](https://app.codecov.io/gh/apache/datafusion-sqlparser-rs/pull/2316?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #2316 +/- ##
==========================================
- Coverage 80.96% 80.95% -0.02%
==========================================
Files 42 42
Lines 33359 33565 +206
Branches 33359 33565 +206
==========================================
+ Hits 27009 27171 +162
- Misses 2790 2800 +10
- Partials 3560 3594 +34
```
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/datafusion-sqlparser-rs/pull/2316?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] PostgreSQL: add support for CREATE AGGREGATE [datafusion-sqlparser-rs]
Posted by "fmguerreiro (via GitHub)" <gi...@apache.org>.
fmguerreiro commented on code in PR #2316:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2316#discussion_r4104410003
##########
src/parser/mod.rs:
##########
@@ -5317,13 +5317,15 @@ 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 self.parse_keyword(Keyword::SCHEMA) {
self.parse_create_schema(or_replace)
} else if self.parse_keyword(Keyword::WAREHOUSE) {
self.parse_create_warehouse(or_replace).map(Into::into)
} else if or_replace {
self.expected_ref(
- "[EXTERNAL] TABLE or [MATERIALIZED] VIEW or FUNCTION or SCHEMA or WAREHOUSE after CREATE OR REPLACE",
+ "[EXTERNAL] TABLE or [MATERIALIZED] VIEW or FUNCTION or AGGREGATE or SCHEMA or WAREHOUSE after CREATE OR REPLACE",
Review Comment:
covered by `parse_create_aggregate_or_replace_with_parallel`.
##########
src/parser/mod.rs:
##########
@@ -7460,6 +7453,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()?),
+ 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());
+ }
+ })
+ }
+
+ /// Parse the value of `FINALFUNC_MODIFY` / `MFINALFUNC_MODIFY`.
+ fn parse_aggregate_modify_kind(&mut self) -> Result<AggregateModifyKind, ParserError> {
+ 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(
Review Comment:
covered across the `parse_create_aggregate_*` tests, including legacy, star, empty-args, and the malformed, unclosed, and duplicate error paths.
##########
src/ast/spans.rs:
##########
@@ -2528,6 +2529,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())),
+ }
+ }
+}
+
+impl Spanned for OperateFunctionArg {
+ fn span(&self) -> Span {
+ union_spans(
+ self.name
+ .iter()
+ .map(|name| name.span)
+ .chain(self.default_expr.iter().map(|expr| expr.span())),
+ )
+ }
+}
+
+impl Spanned for CreateAggregateOption {
+ fn span(&self) -> Span {
+ match self {
+ CreateAggregateOption::StateTransitionFunction(name)
+ | CreateAggregateOption::FinalFunction(name)
Review Comment:
the span's pinned now by `parse_create_aggregate_span`, from the name through the last option value.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] PostgreSQL: add support for CREATE AGGREGATE [datafusion-sqlparser-rs]
Posted by "LucaCappelletti94 (via GitHub)" <gi...@apache.org>.
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] PostgreSQL: add support for CREATE AGGREGATE [datafusion-sqlparser-rs]
Posted by "LucaCappelletti94 (via GitHub)" <gi...@apache.org>.
LucaCappelletti94 commented on code in PR #2316:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2316#discussion_r4016227637
##########
src/ast/spans.rs:
##########
@@ -2518,6 +2519,12 @@ impl Spanned for AlterTable {
}
}
+impl Spanned for CreateAggregate {
+ fn span(&self) -> Span {
+ Span::empty()
+ }
+}
+
Review Comment:
I believe this object contains spanned objects, therefore you can return a span for it.
##########
src/ast/mod.rs:
##########
@@ -10253,16 +10260,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())
+ }
+}
+
Review Comment:
What is the reason for this edit?
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] PostgreSQL: add support for CREATE AGGREGATE [datafusion-sqlparser-rs]
Posted by "fmguerreiro (via GitHub)" <gi...@apache.org>.
fmguerreiro commented on PR #2316:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2316#issuecomment-5853351779
cool, didnt know about the fuzzing in the codebase. added your five seeds in 41ef183d. ran fuzz_parse_roundtrip and fuzz_postgres_accepts over them, both clean.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] PostgreSQL: add support for CREATE AGGREGATE [datafusion-sqlparser-rs]
Posted by "fmguerreiro (via GitHub)" <gi...@apache.org>.
fmguerreiro commented on code in PR #2316:
URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2316#discussion_r4333892611
##########
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:
fixed in ea2bfe88.
##########
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:
fixed in ea2bfe88.
##########
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:
fixed in ea2bfe88.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org