sqlparser: Support `ORDER BY` in functions

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
 

sqlparser/lib/src/analysis/steps/prepare_ast.dart

@@ -19,6 +19,22 @@ class AstPreparingVisitor extends RecursiveVisitor<void, void> {
     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<void, void> {
 
     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);
+  }
 }

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<A>(Transformer<A> 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<A, R>(AstVisitor<A, R> visitor, A arg) {

sqlparser/lib/src/ast/visitor.dart

@@ -505,7 +505,7 @@ class RecursiveVisitor<A, R> implements AstVisitor<A, R?> {
 
   @override
   R? visitWindowFunctionInvocation(WindowFunctionInvocation e, A arg) {
-    return visitExpressionInvocation(e, arg);
+    return visitAggregateFunctionInvocation(e, arg);
   }
 
   @override

sqlparser/lib/src/engine/options.dart

@@ -93,6 +93,10 @@ class SqliteVersion implements Comparable<SqliteVersion> {
   /// 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<SqliteVersion> {
   /// 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.
   ///

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);
     }

sqlparser/lib/utils/node_to_text.dart

@@ -39,6 +39,7 @@ class NodeSqlBuilder extends AstVisitor<void, void> {
 
     symbol('(');
     visit(e.parameters, arg);
+    visitNullable(e.orderBy, arg);
     symbol(')');
 
     if (e.filter != null) {

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');
+  });
 }

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();
+  });
 }

sqlparser/test/parser/partition_test.dart

@@ -65,6 +65,19 @@ final Map<String, Expression> _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() {

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, () {