Merge branch 'simolus3:develop' into rtree

sqlparser/CHANGELOG.md

@@ -1,3 +1,9 @@
+## 0.23.2-dev
+
+- Support resolving the `fts5vocab` module when `fts5` is enabled - thanks to
+  [@FaFre](https://github.com/FaFre).
+- Improve references resolving across subqueries.
+
 ## 0.23.1
 
 - Gracefully handle tokenizer errors related to `@` or `$` variables.

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

@@ -24,6 +24,13 @@ mixin Referencable {
 abstract class ReferenceScope {
   RootScope get rootScope;
 
+  /// All available result sets that can also be seen in child scopes.
+  ///
+  /// Usually, this is the same list as the result sets being declared in this
+  /// scope. However, some exceptions apply (see e.g. [SubqueryInFromScope]).
+  Iterable<ResultSetAvailableInStatement> get resultSetAvailableToChildScopes =>
+      const Iterable.empty();
+
   /// The list of column to which a `*` would expand to.
   ///
   /// This is not necessary the same list of columns that could be resolved
@@ -101,6 +108,30 @@ class RootScope extends ReferenceScope {
   final Map<String, Module> knownModules = CaseInsensitiveMap();
 }
 
+mixin _HasParentScope on ReferenceScope {
+  ReferenceScope get _parentScopeForLookups;
+
+  @override
+  RootScope get rootScope => _parentScopeForLookups.rootScope;
+
+  @override
+  Iterable<ResultSetAvailableInStatement> get resultSetAvailableToChildScopes =>
+      _parentScopeForLookups.resultSetAvailableToChildScopes;
+
+  @override
+  ResultSetAvailableInStatement? resolveResultSet(String name) =>
+      _parentScopeForLookups.resolveResultSet(name);
+
+  @override
+  ResultSet? resolveResultSetToAdd(String name) =>
+      _parentScopeForLookups.resolveResultSetToAdd(name);
+
+  @override
+  List<Column> resolveUnqualifiedReference(String columnName,
+          {bool allowReferenceToResultColumn = false}) =>
+      _parentScopeForLookups.resolveUnqualifiedReference(columnName);
+}
+
 /// A scope used by statements.
 ///
 /// Tables added from `FROM` clauses are added to [resultSets], CTEs are added
@@ -118,9 +149,12 @@ class RootScope extends ReferenceScope {
 ///    [SubqueryInFromScope] is insertted as an intermediatet scope to prevent
 ///    the inner scope from seeing the outer columns.
 
-class StatementScope extends ReferenceScope {
+class StatementScope extends ReferenceScope with _HasParentScope {
   final ReferenceScope parent;
 
+  @override
+  get _parentScopeForLookups => parent;
+
   /// Additional tables (that haven't necessarily been added in a `FROM` clause
   /// that are only visible in this scope).
   ///
@@ -153,29 +187,17 @@ class StatementScope extends ReferenceScope {
 
   StatementScope(this.parent);
 
-  StatementScope? get parentStatementScope {
-    final parent = this.parent;
-    if (parent is StatementScope) {
-      return parent;
-    } else if (parent is MiscStatementSubScope) {
-      return parent.parent;
-    } else {
-      return null;
-    }
+  @override
+  Iterable<ResultSetAvailableInStatement> get resultSetAvailableToChildScopes {
+    return allAvailableResultSets;
   }
 
   /// All result sets available in this and parent scopes.
   Iterable<ResultSetAvailableInStatement> get allAvailableResultSets {
     final here = resultSets.values;
-    final parent = parentStatementScope;
-    return parent != null
-        ? here.followedBy(parent.allAvailableResultSets)
-        : here;
+    return parent.resultSetAvailableToChildScopes.followedBy(here);
   }
 
-  @override
-  RootScope get rootScope => parent.rootScope;
-
   @override
   void addAlias(AstNode origin, ResultSet resultSet, String alias) {
     final createdAlias = TableAlias(resultSet, alias);
@@ -183,22 +205,20 @@ class StatementScope extends ReferenceScope {
     resultSets[alias] = ResultSetAvailableInStatement(origin, createdAlias);
   }
 
-  @override
-  ResultSetAvailableInStatement? resolveResultSet(String name) {
-    return resultSets[name] ?? parentStatementScope?.resolveResultSet(name);
-  }
-
   @override
   void addResolvedResultSet(
       String? name, ResultSetAvailableInStatement resultSet) {
     resultSets[name] = resultSet;
   }
 
+  @override
+  ResultSetAvailableInStatement? resolveResultSet(String name) {
+    return resultSets[name] ?? parent.resolveResultSet(name);
+  }
+
   @override
   ResultSet? resolveResultSetToAdd(String name) {
-    return additionalKnownTables[name] ??
-        parentStatementScope?.resolveResultSetToAdd(name) ??
-        rootScope.knownTables[name];
+    return additionalKnownTables[name] ?? parent.resolveResultSetToAdd(name);
   }
 
   @override
@@ -212,43 +232,28 @@ class StatementScope extends ReferenceScope {
       }
     }
 
-    StatementScope? currentScope = this;
+    final available = resultSets.values;
+    final sourceColumns = <Column>{};
+    final availableColumns = <AvailableColumn>[];
 
-    // Search scopes for a matching column in an added result set. If a column
-    // reference is found in a closer scope, it takes precedence over outer
-    // scopes. However, it's an error if two columns with the same name are
-    // found in the same scope.
-    while (currentScope != null) {
-      final available = currentScope.resultSets.values;
-      final sourceColumns = <Column>{};
-      final availableColumns = <AvailableColumn>[];
+    for (final availableSource in available) {
+      final resolvedColumns =
+          availableSource.resultSet.resultSet?.resolvedColumns;
+      if (resolvedColumns == null) continue;
 
-      for (final availableSource in available) {
-        final resolvedColumns =
-            availableSource.resultSet.resultSet?.resolvedColumns;
-        if (resolvedColumns == null) continue;
-
-        for (final column in resolvedColumns) {
-          if (column.name.toLowerCase() == columnName.toLowerCase() &&
-              sourceColumns.add(column)) {
-            availableColumns.add(AvailableColumn(column, availableSource));
-          }
+      for (final column in resolvedColumns) {
+        if (column.name.toLowerCase() == columnName.toLowerCase() &&
+            sourceColumns.add(column)) {
+          availableColumns.add(AvailableColumn(column, availableSource));
         }
       }
-
-      if (availableColumns.isEmpty) {
-        currentScope = currentScope.parentStatementScope;
-        if (currentScope == null) {
-          // Reached the outermost scope without finding a reference target.
-          return const [];
-        }
-        continue;
-      } else {
-        return availableColumns;
-      }
     }
 
-    return const [];
+    if (availableColumns.isEmpty) {
+      return parent.resolveUnqualifiedReference(columnName);
+    } else {
+      return availableColumns;
+    }
   }
 
   factory StatementScope.forStatement(RootScope root, Statement statement) {
@@ -269,13 +274,24 @@ class StatementScope extends ReferenceScope {
 
 /// A special intermediate scope used for subqueries appearing in a `FROM`
 /// clause so that the subquery can't see outer columns and tables being added.
-class SubqueryInFromScope extends ReferenceScope {
+class SubqueryInFromScope extends ReferenceScope with _HasParentScope {
   final StatementScope enclosingStatement;
 
   SubqueryInFromScope(this.enclosingStatement);
 
   @override
   RootScope get rootScope => enclosingStatement.rootScope;
+
+  // This scope can't see elements from the enclosing statement, but it can see
+  // elements from grandparents.
+  @override
+  ReferenceScope get _parentScopeForLookups => enclosingStatement.parent;
+
+  @override
+  ResultSet? resolveResultSetToAdd(String name) {
+    // CTEs from the enclosing statement are also available here
+    return enclosingStatement.resolveResultSetToAdd(name);
+  }
 }
 
 /// A rarely used sub-scope for AST nodes that belong to a statement, but may
@@ -283,9 +299,12 @@ class SubqueryInFromScope extends ReferenceScope {
 ///
 /// For instance, the body of an `ON CONFLICT DO UPDATE`-clause may refer to a
 /// table alias `excluded` to get access to a conflicting table.
-class MiscStatementSubScope extends ReferenceScope {
+class MiscStatementSubScope extends ReferenceScope with _HasParentScope {
   final StatementScope parent;
 
+  @override
+  get _parentScopeForLookups => parent;
+
   final Map<String?, ResultSetAvailableInStatement> additionalResultSets =
       CaseInsensitiveMap();
 
@@ -304,12 +323,6 @@ class MiscStatementSubScope extends ReferenceScope {
       String? name, ResultSetAvailableInStatement resultSet) {
     additionalResultSets[name] = resultSet;
   }
-
-  @override
-  List<Column> resolveUnqualifiedReference(String columnName,
-      {bool allowReferenceToResultColumn = false}) {
-    return parent.resolveUnqualifiedReference(columnName);
-  }
 }
 
 /// A reference scope that only allows a single added result set.

sqlparser/lib/src/engine/module/fts5.dart

@@ -6,6 +6,7 @@ class Fts5Extension implements Extension {
   @override
   void register(SqlEngine engine) {
     engine.registerModule(_Fts5Module());
+    engine.registerModule(_Fts5VocabModule());
     engine.registerFunctionHandler(const _Fts5Functions());
   }
 }
@@ -35,6 +36,59 @@ class _Fts5Module extends Module {
   }
 }
 
+class _Fts5VocabModule extends Module {
+  _Fts5VocabModule() : super('fts5vocab');
+
+  @override
+  Table parseTable(CreateVirtualTableStatement stmt) {
+    if (stmt.argumentContent.length < 2 || stmt.argumentContent.length > 3) {
+      throw ArgumentError('''
+fts5vocab table requires at least 
+two arguments (<referenced fts5 table name>, <type>) 
+and maximum three arguments when using an attached database
+ ''');
+    }
+
+    final type = stmt.argumentContent.last.replaceAll(RegExp(r'''["' ]'''), '');
+    switch (type) {
+      case 'row':
+        return Table(
+            name: stmt.tableName,
+            resolvedColumns: [
+              TableColumn('term', const ResolvedType(type: BasicType.text)),
+              TableColumn('doc', const ResolvedType(type: BasicType.int)),
+              TableColumn('cnt', const ResolvedType(type: BasicType.int)),
+            ],
+            definition: stmt,
+            isVirtual: true);
+      case 'col':
+        return Table(
+            name: stmt.tableName,
+            resolvedColumns: [
+              TableColumn('term', const ResolvedType(type: BasicType.text)),
+              TableColumn('col', const ResolvedType(type: BasicType.text)),
+              TableColumn('doc', const ResolvedType(type: BasicType.int)),
+              TableColumn('cnt', const ResolvedType(type: BasicType.int)),
+            ],
+            definition: stmt,
+            isVirtual: true);
+      case 'instance':
+        return Table(
+            name: stmt.tableName,
+            resolvedColumns: [
+              TableColumn('term', const ResolvedType(type: BasicType.text)),
+              TableColumn('doc', const ResolvedType(type: BasicType.int)),
+              TableColumn('col', const ResolvedType(type: BasicType.text)),
+              TableColumn('offset', const ResolvedType(type: BasicType.int)),
+            ],
+            definition: stmt,
+            isVirtual: true);
+      default:
+        throw ArgumentError('Unknown fts5vocab table type');
+    }
+  }
+}
+
 class _Fts5Table extends Table {
   _Fts5Table(
       {required String name,

sqlparser/pubspec.yaml

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

sqlparser/test/analysis/reference_resolver_test.dart

@@ -163,13 +163,78 @@ void main() {
     });
   });
 
-  test('resolves sub-queries', () {
-    final engine = SqlEngine()..registerTable(demoTable);
+  group('sub-queries', () {
+    test('are resolved', () {
+      final engine = SqlEngine()..registerTable(demoTable);
 
-    final context = engine.analyze(
-        'SELECT d.*, (SELECT id FROM demo WHERE id = d.id) FROM demo d;');
+      final context = engine.analyze(
+          'SELECT d.*, (SELECT id FROM demo WHERE id = d.id) FROM demo d;');
 
-    expect(context.errors, isEmpty);
+      expect(context.errors, isEmpty);
+    });
+
+    test('cannot refer to outer tables if used in FROM', () {
+      final engine = SqlEngine()..registerTable(demoTable);
+
+      final context = engine.analyze(
+          'SELECT d.* FROM demo d, (SELECT * FROM demo WHERE id = d.id);');
+
+      context.expectError('d.id',
+          type: AnalysisErrorType.referencedUnknownTable);
+    });
+
+    test('can refer to CTEs if used in FROM', () {
+      final engine = SqlEngine()..registerTable(demoTable);
+
+      final context = engine.analyze('WITH cte AS (SELECT * FROM demo) '
+          'SELECT d.* FROM demo d, (SELECT * FROM cte);');
+
+      expect(context.errors, isEmpty);
+    });
+
+    test('can nest and see outer tables if that is a subquery', () {
+      final engine = SqlEngine()..registerTable(demoTable);
+
+      final context = engine.analyze('''
+SELECT
+  (SELECT *
+   FROM
+     demo "inner",
+     (SELECT * FROM demo WHERE "inner".id = "outer".id)
+  )
+  FROM demo "outer";
+''');
+
+      // note that "outer".id is visible and should not report an error
+      context.expectError('"inner".id',
+          type: AnalysisErrorType.referencedUnknownTable);
+    });
+
+    test('nested via FROM cannot see outer result sets', () {
+      final engine = SqlEngine()..registerTable(demoTable);
+
+      final context = engine.analyze('''
+SELECT *
+  FROM
+    demo "outer",
+    (SELECT * FROM demo "inner",
+      (SELECT * FROM demo WHERE "inner".id = "outer".id))
+''');
+
+      expect(
+        context.errors,
+        [
+          analysisErrorWith(
+            lexeme: '"inner".id',
+            type: AnalysisErrorType.referencedUnknownTable,
+          ),
+          analysisErrorWith(
+            lexeme: '"outer".id',
+            type: AnalysisErrorType.referencedUnknownTable,
+          ),
+        ],
+      );
+    });
   });
 
   test('resolves sub-queries as data sources', () {

sqlparser/test/analysis/regression_test.dart

@@ -188,4 +188,32 @@ WHERE EXISTS(SELECT *
 
     expect(result.errors, isEmpty);
   });
+
+  test('regression test for #2010', () {
+    // https://github.com/simolus3/drift/issues/2010
+    final engine = SqlEngine()
+      ..registerTableFromSql('CREATE TABLE place_hierarchy (feature_id INT);')
+      ..registerTableFromSql('CREATE TABLE place_name (feature_id INT);')
+      ..registerTableFromSql('CREATE TABLE place (feature_id INT);');
+
+    final result = engine.analyze('''
+  SELECT
+    (
+      SELECT
+        true
+      FROM (
+        SELECT
+          'test' AS name
+        FROM
+          place_hierarchy ph
+        where
+          ph.feature_id = p.feature_id
+      ) second_select
+    ) first_select
+  FROM place_name matches
+  INNER JOIN place p ON p.feature_id = matches.feature_id;
+''');
+
+    expect(result.errors, isEmpty);
+  });
 }

sqlparser/test/engine/module/fts5_test.dart

@@ -20,6 +20,48 @@ void main() {
       expect(columns.single.name, 'bar');
     });
 
+    group('creating fts5vocab tables', () {
+      final engine = SqlEngine(_fts5Options);
+
+      test('can create fts5vocab instance table', () {
+        final result = engine.analyze('CREATE VIRTUAL TABLE foo USING '
+            'fts5vocab(bar, instance)');
+
+        final table = const SchemaFromCreateTable()
+            .read(result.root as TableInducingStatement);
+
+        expect(table.name, 'foo');
+        final columns = table.resultColumns;
+        expect(columns, hasLength(4));
+        expect(columns.map((e) => e.name), contains('offset'));
+      });
+
+      test('can create fts5vocab row table', () {
+        final result = engine.analyze('CREATE VIRTUAL TABLE foo USING '
+            'fts5vocab(bar, row)');
+
+        final table = const SchemaFromCreateTable()
+            .read(result.root as TableInducingStatement);
+
+        expect(table.name, 'foo');
+        final columns = table.resultColumns;
+        expect(columns, hasLength(3));
+      });
+
+      test('can create fts5vocab col table', () {
+        final result = engine.analyze('CREATE VIRTUAL TABLE foo USING '
+            'fts5vocab(bar, col)');
+
+        final table = const SchemaFromCreateTable()
+            .read(result.root as TableInducingStatement);
+
+        expect(table.name, 'foo');
+        final columns = table.resultColumns;
+        expect(columns, hasLength(4));
+        expect(columns.map((e) => e.name), contains('col'));
+      });
+    });
+
     test('handles the UNINDEXED column option', () {
       final result = engine
           .analyze('CREATE VIRTUAL TABLE foo USING fts5(bar, baz UNINDEXED)');