diff --git a/sqlparser/CHANGELOG.md b/sqlparser/CHANGELOG.md index 67e179a4..73ebef5e 100644 --- a/sqlparser/CHANGELOG.md +++ b/sqlparser/CHANGELOG.md @@ -1,6 +1,8 @@ ## 0.32.1-dev - Treat the result of `sum()` as nullable when inferring types. +- Support features added in sqlite 3.44: + - `ORDER BY` clauses as part of aggregate functions. ## 0.32.0 diff --git a/sqlparser/lib/src/analysis/steps/prepare_ast.dart b/sqlparser/lib/src/analysis/steps/prepare_ast.dart index 4f56235a..493350b9 100644 --- a/sqlparser/lib/src/analysis/steps/prepare_ast.dart +++ b/sqlparser/lib/src/analysis/steps/prepare_ast.dart @@ -19,6 +19,22 @@ class AstPreparingVisitor extends RecursiveVisitor { resolveIndexOfVariables(_foundVariables); } + @override + void visitAggregateFunctionInvocation( + AggregateFunctionInvocation e, void arg) { + if (e.orderBy != null && + context.engineOptions.version < SqliteVersion.v3_44) { + context.reportError(AnalysisError( + type: AnalysisErrorType.notSupportedInDesiredVersion, + message: + 'ORDER BY in aggregate functions require sqlite 3.44 or later.', + relevantNode: e.orderBy, + )); + } + + super.visitAggregateFunctionInvocation(e, arg); + } + @override void defaultInsertSource(InsertSource e, void arg) { e.scope = SourceScope(e.parent!.statementScope); @@ -225,4 +241,20 @@ class AstPreparingVisitor extends RecursiveVisitor { super.visitDriftSpecificNode(e, arg); } + + @override + void visitWindowFunctionInvocation(WindowFunctionInvocation e, void arg) { + // Window functions can't use ORDER BY in their arguments: + // https://github.com/sqlite/sqlite/blob/85b1f5c2f6a05ba151496122fc62b10d560498ca/src/expr.c#L1231-L1235 + if (e.orderBy != null) { + context.reportError(AnalysisError( + type: AnalysisErrorType.synctactic, + message: + 'Window functions may not use `ORDER BY` in their parameter list', + relevantNode: e.orderBy, + )); + } + + super.visitWindowFunctionInvocation(e, arg); + } } diff --git a/sqlparser/lib/src/ast/expressions/aggregate.dart b/sqlparser/lib/src/ast/expressions/aggregate.dart index b107cce0..8f365319 100644 --- a/sqlparser/lib/src/ast/expressions/aggregate.dart +++ b/sqlparser/lib/src/ast/expressions/aggregate.dart @@ -12,6 +12,7 @@ class AggregateFunctionInvocation extends Expression @override FunctionParameters parameters; + OrderByBase? orderBy; Expression? filter; @override @@ -20,6 +21,7 @@ class AggregateFunctionInvocation extends Expression AggregateFunctionInvocation({ required this.function, required this.parameters, + this.orderBy, this.filter, }) : nameToken = function; @@ -31,6 +33,7 @@ class AggregateFunctionInvocation extends Expression @override void transformChildren(Transformer transformer, A arg) { parameters = transformer.transformChild(parameters, this, arg); + orderBy = transformer.transformNullableChild(orderBy, this, arg); filter = transformer.transformNullableChild(filter, this, arg); } @@ -61,14 +64,14 @@ class WindowFunctionInvocation extends AggregateFunctionInvocation { final String? windowName; WindowFunctionInvocation( - {required IdentifierToken function, - required FunctionParameters parameters, - Expression? filter, + {required super.function, + required super.parameters, + super.orderBy, + super.filter, this.windowDefinition, this.windowName}) // one of window definition or name must be null - : assert((windowDefinition == null) || (windowName == null)), - super(function: function, parameters: parameters, filter: filter); + : assert((windowDefinition == null) || (windowName == null)); @override R accept(AstVisitor visitor, A arg) { diff --git a/sqlparser/lib/src/ast/visitor.dart b/sqlparser/lib/src/ast/visitor.dart index 48adfb97..3e0f4232 100644 --- a/sqlparser/lib/src/ast/visitor.dart +++ b/sqlparser/lib/src/ast/visitor.dart @@ -505,7 +505,7 @@ class RecursiveVisitor implements AstVisitor { @override R? visitWindowFunctionInvocation(WindowFunctionInvocation e, A arg) { - return visitExpressionInvocation(e, arg); + return visitAggregateFunctionInvocation(e, arg); } @override diff --git a/sqlparser/lib/src/engine/options.dart b/sqlparser/lib/src/engine/options.dart index 8ef86010..edcfe26b 100644 --- a/sqlparser/lib/src/engine/options.dart +++ b/sqlparser/lib/src/engine/options.dart @@ -93,6 +93,10 @@ class SqliteVersion implements Comparable { /// can't provide analysis warnings when using recent sqlite3 features. static const SqliteVersion minimum = SqliteVersion.v3(34); + /// Version `3.44.0` has added `ORDER BY` clauses as parameters for aggregate + /// functions and more SQL functions. + static const SqliteVersion v3_44 = SqliteVersion.v3(44); + /// Version `3.43.0` added the built-in `timediff` and `octet_length` /// functions. static const SqliteVersion v3_43 = SqliteVersion.v3(43); @@ -118,7 +122,7 @@ class SqliteVersion implements Comparable { /// The highest sqlite version supported by this `sqlparser` package. /// /// Newer features in `sqlite3` may not be recognized by this library. - static const SqliteVersion current = v3_43; + static const SqliteVersion current = v3_44; /// The major version of sqlite. /// diff --git a/sqlparser/lib/src/reader/parser.dart b/sqlparser/lib/src/reader/parser.dart index 919eb6ee..51f74f84 100644 --- a/sqlparser/lib/src/reader/parser.dart +++ b/sqlparser/lib/src/reader/parser.dart @@ -922,11 +922,17 @@ class Parser { } else if (_matchOne(TokenType.leftParen)) { // We have something like "foo(" -> that's a function! final parameters = _functionParameters(); + + // Aggregate functions can use `ORDER BY` in their argument list. + final orderBy = _orderBy(); + final rightParen = _consume( TokenType.rightParen, 'Expected closing bracket after argument list'); - if (_peek.type == TokenType.filter || _peek.type == TokenType.over) { - return _aggregate(first, parameters); + if (orderBy != null || + _peek.type == TokenType.filter || + _peek.type == TokenType.over) { + return _aggregate(first, parameters, orderBy); } return FunctionExpression(name: first.identifier, parameters: parameters) @@ -980,7 +986,10 @@ class Parser { } AggregateFunctionInvocation _aggregate( - IdentifierToken name, FunctionParameters params) { + IdentifierToken name, + FunctionParameters params, + OrderByBase? orderBy, + ) { Expression? filter; // https://www.sqlite.org/syntax/filter.html (it's optional) @@ -1005,6 +1014,7 @@ class Parser { return WindowFunctionInvocation( function: name, parameters: params, + orderBy: orderBy, filter: filter, windowDefinition: window, windowName: windowName, @@ -1013,6 +1023,7 @@ class Parser { return AggregateFunctionInvocation( function: name, parameters: params, + orderBy: orderBy, filter: filter, )..setSpan(name, _previous); } diff --git a/sqlparser/lib/utils/node_to_text.dart b/sqlparser/lib/utils/node_to_text.dart index c54ac376..229a9211 100644 --- a/sqlparser/lib/utils/node_to_text.dart +++ b/sqlparser/lib/utils/node_to_text.dart @@ -39,6 +39,7 @@ class NodeSqlBuilder extends AstVisitor { symbol('('); visit(e.parameters, arg); + visitNullable(e.orderBy, arg); symbol(')'); if (e.filter != null) { diff --git a/sqlparser/test/analysis/errors/syntax_error_test.dart b/sqlparser/test/analysis/errors/syntax_error_test.dart index e9ba8041..0b29c60e 100644 --- a/sqlparser/test/analysis/errors/syntax_error_test.dart +++ b/sqlparser/test/analysis/errors/syntax_error_test.dart @@ -128,4 +128,14 @@ END; }); }); }); + + test('window function with order by', () { + final engine = SqlEngine(EngineOptions(version: SqliteVersion.v3_44)) + ..registerTable(demoTable); + + engine + .analyze('SELECT group_concat(content ORDER BY id DESC) ' + 'OVER (ROWS UNBOUNDED PRECEDING) FROM demo;') + .expectError('ORDER BY id DESC'); + }); } diff --git a/sqlparser/test/analysis/errors/unsupported_test.dart b/sqlparser/test/analysis/errors/unsupported_test.dart index f5d54bdf..123831fe 100644 --- a/sqlparser/test/analysis/errors/unsupported_test.dart +++ b/sqlparser/test/analysis/errors/unsupported_test.dart @@ -155,4 +155,12 @@ void main() { currentEngine.analyze(right).expectNoError(); currentEngine.analyze(full).expectNoError(); }); + + test('warns about aggregate functions with order by', () { + const sql = "SELECT group_concat(content ORDER BY id) FROM demo"; + + minimumEngine.analyze(sql).expectError('ORDER BY id', + type: AnalysisErrorType.notSupportedInDesiredVersion); + currentEngine.analyze(sql).expectNoError(); + }); } diff --git a/sqlparser/test/parser/partition_test.dart b/sqlparser/test/parser/partition_test.dart index 2c595cd0..ce3ebbf7 100644 --- a/sqlparser/test/parser/partition_test.dart +++ b/sqlparser/test/parser/partition_test.dart @@ -65,6 +65,19 @@ final Map _testCases = { BooleanLiteral(true), ), ), + "string_agg(foo, ', ' ORDER BY foo DESC)": AggregateFunctionInvocation( + function: identifier('string_agg'), + parameters: ExprFunctionParameters(parameters: [ + Reference(columnName: 'foo'), + StringLiteral(', '), + ]), + orderBy: OrderBy(terms: [ + OrderingTerm( + expression: Reference(columnName: 'foo'), + orderingMode: OrderingMode.descending, + ), + ]), + ), }; void main() { diff --git a/sqlparser/test/utils/node_to_text_test.dart b/sqlparser/test/utils/node_to_text_test.dart index 184f9c6e..9c221cbf 100644 --- a/sqlparser/test/utils/node_to_text_test.dart +++ b/sqlparser/test/utils/node_to_text_test.dart @@ -82,7 +82,7 @@ END; test('after update of when', () { testFormat(''' -CREATE TRIGGER IF NOT EXISTS my_trigger AFTER UPDATE OF c1, c2 ON t1 +CREATE TRIGGER IF NOT EXISTS my_trigger AFTER UPDATE OF c1, c2 ON t1 WHEN foo = bar BEGIN SELECT * FROM t2; @@ -309,6 +309,15 @@ CREATE UNIQUE INDEX my_idx ON t1 (c1, c2, c3) WHERE c1 < c3; '''); }); + test('aggregate with order by', () { + testFormat(''' + SELECT + string_agg(foo, ',' ORDER BY foo DESC, bar), + string_agg(foo, ',' ORDER BY foo DESC) FILTER (WHERE foo > 10) + FROM bar + '''); + }); + group('joins', () { for (final kind in ['LEFT', 'RIGHT', 'FULL']) { test(kind, () {