Improve table analysis, parse key ordering (#1007)

moor/test/data/tables/custom_tables.g.dart

@@ -1062,7 +1062,7 @@ class Mytable extends Table with TableInfo<Mytable, MytableData> {
   late final GeneratedIntColumn someid = _constructSomeid();
   GeneratedIntColumn _constructSomeid() {
     return GeneratedIntColumn('someid', $tableName, false,
-        declaredAsPrimaryKey: true, $customConstraints: 'NOT NULL PRIMARY KEY');
+        $customConstraints: 'NOT NULL');
   }
 
   final VerificationMeta _sometextMeta = const VerificationMeta('sometext');
@@ -1136,6 +1136,8 @@ class Mytable extends Table with TableInfo<Mytable, MytableData> {
     return Mytable(_db, alias);
   }
 
+  @override
+  List<String> get customConstraints => const ['PRIMARY KEY (someid DESC)'];
   @override
   bool get dontWriteConstraints => true;
 }

moor/test/data/tables/tables.moor

@@ -27,10 +27,11 @@ create table config (
 CREATE INDEX IF NOT EXISTS value_idx ON config (config_value);
 
 CREATE TABLE mytable (
-    someid INTEGER NOT NULL PRIMARY KEY,
+    someid INTEGER NOT NULL,
     sometext TEXT,
     is_inserting BOOLEAN,
-    somedate DATETIME
+    somedate DATETIME,
+    PRIMARY KEY (someid DESC)
 );
 
 CREATE VIRTUAL TABLE email USING fts5(sender, title, body) AS EMail;

moor/test/parsed_sql/moor_files_integration_test.dart

@@ -26,10 +26,12 @@ const _createConfig = 'CREATE TABLE IF NOT EXISTS config ('
     'sync_state_implicit INTEGER);';
 
 const _createMyTable = 'CREATE TABLE IF NOT EXISTS mytable ('
-    'someid INTEGER NOT NULL PRIMARY KEY, '
+    'someid INTEGER NOT NULL, '
     'sometext TEXT, '
     'is_inserting INTEGER, '
-    'somedate INTEGER);';
+    'somedate INTEGER, '
+    'PRIMARY KEY (someid DESC)'
+    ');';
 
 const _createEmail = 'CREATE VIRTUAL TABLE IF NOT EXISTS email USING '
     'fts5(sender, title, body);';
@@ -85,6 +87,7 @@ void main() {
     expect(db.noIds.primaryKey, [db.noIds.payload]);
     expect(db.withDefaults.primaryKey, isEmpty);
     expect(db.config.primaryKey, [db.config.configKey]);
+    expect(db.mytable.primaryKey, [db.mytable.someid]);
   });
 
   test('supports absent values for primary key integers', () async {

moor_generator/lib/src/analyzer/moor/create_table_reader.dart

@@ -184,11 +184,13 @@ class CreateTableReader {
 
     for (final keyConstraint in table.tableConstraints.whereType<KeyClause>()) {
       if (keyConstraint.isPrimaryKey) {
-        primaryKeyFromTableConstraint = {
-          for (final column in keyConstraint.indexedColumns)
-            if (foundColumns.containsKey(column.columnName))
-              foundColumns[column.columnName]
-        };
+        primaryKeyFromTableConstraint = {};
+        for (final column in keyConstraint.columns) {
+          final expr = column.expression;
+          if (expr is Reference && foundColumns.containsKey(expr.columnName)) {
+            primaryKeyFromTableConstraint.add(foundColumns[expr.columnName]);
+          }
+        }
       }
     }
 

moor_generator/lib/src/analyzer/moor/entity_handler.dart

@@ -31,7 +31,9 @@ class EntityHandler extends BaseAnalyzer {
     for (final entity in file.declaredEntities) {
       if (entity is MoorTable) {
         entity.references.clear();
-        _handleMoorDeclaration<MoorTableDeclaration>(entity, _tables);
+        final node =
+            _handleMoorDeclaration<MoorTableDeclaration>(entity, _tables);
+        _lint(node, entity.sqlName);
       } else if (entity is MoorTrigger) {
         entity.clearResolvedReferences();
 

moor_generator/pubspec.yaml

@@ -24,7 +24,7 @@ dependencies:
   # Moor-specific analysis and apis
   moor: ^4.0.0-nullsafety
   sqlite3: ^0.1.6
-  sqlparser: ^0.12.0-nullsafety.0
+  sqlparser: ^0.13.0-nullsafety.0
 
   # Dart analysis
   analyzer: ">=0.40.0 <0.42.0"

sqlparser/CHANGELOG.md

@@ -1,3 +1,8 @@
+## 0.13.0-nullsafety.0
+
+- Parse ordering in table key constraints
+  - Deprecate `KeyClause.indexedColumns` in favor of `KeyClause.columns`
+
 ## 0.12.0-nullsafety.0
 
 - Migrate to null-safety

sqlparser/lib/src/analysis/schema/column.dart

@@ -61,6 +61,8 @@ class TableColumn extends Column implements ColumnWithType {
   /// The table this column belongs to.
   Table? table;
 
+  late final bool _isAliasForRowId = _computeIsAliasForRowId();
+
   TableColumn(this.name, this._type, {this.definition});
 
   /// Applies a type hint to this column.
@@ -80,6 +82,10 @@ class TableColumn extends Column implements ColumnWithType {
   /// - if this column has a [PrimaryKeyColumn], the [OrderingMode] of that
   ///   constraint is not [OrderingMode.descending].
   bool isAliasForRowId() {
+    return _isAliasForRowId;
+  }
+
+  bool _computeIsAliasForRowId() {
     if (definition == null ||
         table == null ||
         type.type != BasicType.int ||
@@ -93,8 +99,13 @@ class TableColumn extends Column implements ColumnWithType {
     for (final tableConstraint in columnsWithKey) {
       if (!tableConstraint.isPrimaryKey) continue;
 
-      final columns = tableConstraint.indexedColumns;
-      if (columns.length == 1 && columns.single.columnName == name) {
+      final columns = tableConstraint.columns;
+      if (columns.length != 1) continue;
+
+      final singleColumnExpr = columns.single.expression;
+
+      if (singleColumnExpr is Reference &&
+          singleColumnExpr.columnName == name) {
         return true;
       }
     }

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

@@ -63,6 +63,13 @@ class ColumnResolver extends RecursiveVisitor<void, void> {
     visitChildren(e, arg);
   }
 
+  @override
+  void visitTableReference(TableReference e, void arg) {
+    if (e.resolved == null) {
+      _resolveTableReference(e);
+    }
+  }
+
   void _addIfResolved(AstNode node, TableReference ref) {
     final table = _resolveTableReference(ref);
     if (table != null) {

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

@@ -40,6 +40,25 @@ class LintingVisitor extends RecursiveVisitor<void, void> {
     visitChildren(e, arg);
   }
 
+  @override
+  void visitTableConstraint(TableConstraint e, void arg) {
+    if (e is KeyClause && e.isPrimaryKey) {
+      // Primary key clauses may only include simple columns
+      for (final column in e.columns) {
+        final expr = column.expression;
+        if (expr is! Reference || expr.tableName != null) {
+          context.reportError(AnalysisError(
+            type: AnalysisErrorType.synctactic,
+            message: 'Only column names can be used in a PRIMARY KEY clause',
+            relevantNode: expr,
+          ));
+        }
+      }
+    }
+
+    visitChildren(e, arg);
+  }
+
   @override
   void visitTuple(Tuple e, void arg) {
     if (!e.usedAsRowValue) return;

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

@@ -19,6 +19,19 @@ class AstPreparingVisitor extends RecursiveVisitor<void, void> {
     _resolveIndexOfVariables();
   }
 
+  @override
+  void visitCreateTableStatement(CreateTableStatement e, void arg) {
+    final scope = e.scope = e.scope.createChild();
+    final registeredTable = scope.resolve(e.tableName) as Table?;
+
+    if (registeredTable != null) {
+      scope.availableColumns = registeredTable.resolvedColumns;
+      for (final column in registeredTable.resolvedColumns) {
+        scope.register(column.name, column);
+      }
+    }
+  }
+
   @override
   void visitSelectStatement(SelectStatement e, void arg) {
     // a select statement can appear as a sub query which has its own scope, so

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

@@ -6,6 +6,28 @@ class ReferenceResolver extends RecursiveVisitor<void, void> {
 
   ReferenceResolver(this.context);
 
+  @override
+  void visitForeignKeyClause(ForeignKeyClause e, void arg) {
+    final table = e.foreignTable.resultSet;
+    if (table == null) {
+      // If the table wasn't found, an earlier step will have reported an error
+      return super.visitForeignKeyClause(e, arg);
+    }
+
+    for (final columnName in e.columnNames) {
+      _resolveReferenceInTable(columnName, table);
+    }
+  }
+
+  void _resolveReferenceInTable(Reference ref, ResultSet resultSet) {
+    final column = resultSet.findColumn(ref.columnName);
+    if (column == null) {
+      _reportUnknownColumnError(ref, columns: resultSet.resolvedColumns);
+    } else {
+      ref.resolved = column;
+    }
+  }
+
   @override
   void visitReference(Reference e, void arg) {
     if (e.resolved != null) {
@@ -26,12 +48,7 @@ class ReferenceResolver extends RecursiveVisitor<void, void> {
           relevantNode: e,
         ));
       } else {
-        final column = resultSet.findColumn(e.columnName);
-        if (column == null) {
-          _reportUnknownColumnError(e, columns: resultSet.resolvedColumns);
-        } else {
-          e.resolved = column;
-        }
+        _resolveReferenceInTable(e, resultSet);
       }
     } else if (aliasesForRowId.contains(e.columnName.toLowerCase())) {
       // special case for aliases to a rowid

sqlparser/lib/src/ast/schema/table_definition.dart

@@ -84,15 +84,21 @@ abstract class TableConstraint extends AstNode {
 
 class KeyClause extends TableConstraint {
   final bool isPrimaryKey;
-  final List<Reference> indexedColumns;
+  final List<IndexedColumn> columns;
   final ConflictClause? onConflict;
 
   bool get isUnique => !isPrimaryKey;
 
+  @Deprecated('Use columns instead')
+  List<Reference> get indexedColumns {
+    return [
+      for (final column in columns)
+        if (column.expression is Reference) column.expression as Reference
+    ];
+  }
+
   KeyClause(String? name,
-      {required this.isPrimaryKey,
-      required this.indexedColumns,
-      this.onConflict})
+      {required this.isPrimaryKey, required this.columns, this.onConflict})
       : super(name);
 
   @override
@@ -102,11 +108,11 @@ class KeyClause extends TableConstraint {
 
   @override
   void transformChildren<A>(Transformer<A> transformer, A arg) {
-    transformer.transformChildren(indexedColumns, this, arg);
+    transformer.transformChildren(columns, this, arg);
   }
 
   @override
-  Iterable<AstNode> get childNodes => indexedColumns;
+  Iterable<AstNode> get childNodes => columns;
 }
 
 class CheckTable extends TableConstraint {

sqlparser/lib/src/engine/sql_engine.dart

@@ -145,7 +145,7 @@ class SqlEngine {
     final result = parse(sql);
     final analyzed = analyzeParsed(result, stmtOptions: stmtOptions);
 
-    // Add parsing errors that occured to the beginning since they are the most
+    // Add parsing errors that occurred at the beginning since they are the most
     // prominent problems.
     analyzed.errors
         .insertAll(0, result.errors.map((e) => AnalysisError.fromParser(e)));

sqlparser/lib/src/reader/parser/parser.dart

@@ -2249,12 +2249,16 @@ class Parser {
         _consume(TokenType.key, 'Expected KEY to start PRIMARY KEY clause');
       }
 
-      final columns = _listColumnsInParentheses(allowEmpty: false);
+      _consume(TokenType.leftParen,
+          'Expected a left parenthesis to start key columns');
+      final columns = _indexedColumns();
+      _consume(
+          TokenType.rightParen, 'Expected a closing parenthesis after columns');
       final conflictClause = _conflictClauseOrNull();
 
       result = KeyClause(name,
           isPrimaryKey: isPrimaryKey,
-          indexedColumns: columns,
+          columns: columns,
           onConflict: conflictClause);
     } else if (_matchOne(TokenType.check)) {
       final expr = _expressionInParentheses();

sqlparser/lib/utils/node_to_text.dart

@@ -1037,7 +1037,7 @@ class _NodeSqlBuilder extends AstVisitor<void, void> {
       }
 
       _symbol('(');
-      _join(e.indexedColumns, ',');
+      _join(e.columns, ',');
       _symbol(')');
       _conflictClause(e.onConflict);
     } else if (e is CheckTable) {

sqlparser/pubspec.yaml

@@ -1,6 +1,6 @@
 name: sqlparser
 description: Parses sqlite statements and performs static analysis on them
-version: 0.12.0-nullsafety.0
+version: 0.13.0-nullsafety.0
 homepage: https://github.com/simolus3/moor/tree/develop/sqlparser
 #homepage: https://moor.simonbinder.eu/
 issue_tracker: https://github.com/simolus3/moor/issues

sqlparser/test/analysis/errors/syntax_error_test.dart

@@ -1,5 +1,6 @@
 // tests for syntax errors revealed during static analysis.
 
+import 'package:sqlparser/sqlparser.dart';
 import 'package:sqlparser/src/analysis/analysis.dart';
 import 'package:sqlparser/src/engine/sql_engine.dart';
 import 'package:test/test.dart';
@@ -19,4 +20,25 @@ void main() {
               contains('Expected a conflict clause'))
     ]);
   });
+
+  test('complex expressions in PRIMARY KEY clause', () {
+    final engine = SqlEngine();
+    final parseResult = engine.parse('''
+      CREATE TABLE tbl (
+        foo INTEGER NOT NULL,
+        bar TEXT NOT NULL,
+        PRIMARY KEY (foo DESC, bar || 'test')
+      );
+    ''');
+    engine.registerTable(const SchemaFromCreateTable()
+        .read(parseResult.rootNode as CreateTableStatement));
+    final analyzeResult = engine.analyzeParsed(parseResult);
+
+    expect(analyzeResult.errors, [
+      const TypeMatcher<AnalysisError>()
+          .having((e) => e.type, 'type', AnalysisErrorType.synctactic)
+          .having((e) => e.message, 'message',
+              contains('Only column names can be used in a PRIMARY KEY clause'))
+    ]);
+  });
 }

sqlparser/test/parser/create_table_test.dart

@@ -94,9 +94,9 @@ void main() {
           KeyClause(
             null,
             isPrimaryKey: false,
-            indexedColumns: [
-              Reference(columnName: 'score'),
-              Reference(columnName: 'display_name'),
+            columns: [
+              IndexedColumn(Reference(columnName: 'score')),
+              IndexedColumn(Reference(columnName: 'display_name')),
             ],
             onConflict: ConflictClause.abort,
           ),
@@ -125,6 +125,28 @@ void main() {
     );
   });
 
+  test('parses KEY ORDERING in PRIMARY KEY clause', () {
+    testStatement(
+      'CREATE TABLE a (b TEXT, PRIMARY KEY (b DESC))',
+      CreateTableStatement(
+        tableName: 'a',
+        columns: [ColumnDefinition(columnName: 'b', typeName: 'TEXT')],
+        tableConstraints: [
+          KeyClause(
+            null,
+            isPrimaryKey: true,
+            columns: [
+              IndexedColumn(
+                Reference(columnName: 'b'),
+                OrderingMode.descending,
+              ),
+            ],
+          ),
+        ],
+      ),
+    );
+  });
+
   test('parses MAPPED BY constraints when in moor mode', () {
     testStatement(
       'CREATE TABLE a (b NOT NULL MAPPED BY `Mapper()` PRIMARY KEY)',

sqlparser/test/utils/node_to_text_test.dart

@@ -98,7 +98,7 @@ CREATE TABLE IF NOT EXISTS my_table(
     COLLATE c
     REFERENCES t2 (c) ON DELETE RESTRICT ON UPDATE NO ACTION,
 
-  PRIMARY KEY (foo, bar) ON CONFLICT ABORT,
+  PRIMARY KEY (foo, bar DESC) ON CONFLICT ABORT,
   UNIQUE (baz) ON CONFLICT REPLACE,
   CONSTRAINT my_constraint CHECK(baz < 3),
   FOREIGN KEY (foo, baz) REFERENCES t2 ON DELETE SET NULL ON UPDATE CASCADE