Two more tests on scopes

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

@@ -20,9 +20,16 @@ mixin Referencable {
   bool get visibleToChildren => false;
 }
 
+/// A class managing which tables and columns are visible to which AST nodes.
 abstract class ReferenceScope {
   RootScope get rootScope;
 
+  /// The list of column to which a `*` would expand to.
+  ///
+  /// This is not necessary the same list of columns that could be resolved
+  /// through [resolveUnqualifiedReference]. For subquery expressions, columns
+  /// in parent scopes may be referenced without a qualified, but they don't
+  /// appear in a `*` expansion for the subquery.
   List<Column>? get expansionOfStarColumn => null;
 
   /// Attempts to find a result set that has been added to this scope, for
@@ -33,6 +40,10 @@ abstract class ReferenceScope {
   /// `bar` column in that result set).
   ResultSetAvailableInStatement? resolveResultSet(String name) => null;
 
+  /// Adds an added result set to this scope.
+  ///
+  /// This operation is not supported for all kinds of scopes, a [StateError]
+  /// is thrown for invalid scopes.
   void addResolvedResultSet(
       String? name, ResultSetAvailableInStatement resultSet) {
     throw StateError('Result set cannot be added in this scope: $this');
@@ -40,6 +51,9 @@ abstract class ReferenceScope {
 
   /// Registers a [ResultSetAvailableInStatement] to a [TableAlias] for the
   /// given [resultSet].
+  ///
+  /// Like [addResolvedResultSet], this operation is not supported on all
+  /// scopes.
   void addAlias(AstNode origin, ResultSet resultSet, String alias) {
     final createdAlias = TableAlias(resultSet, alias);
     addResolvedResultSet(
@@ -53,25 +67,80 @@ abstract class ReferenceScope {
   /// scope and [resolveResultSet] will find that afterwards.
   ResultSet? resolveResultSetToAdd(String name) => rootScope.knownTables[name];
 
+  /// Attempts to resolve an unqualified reference from a [columnName].
+  ///
+  /// In sqlite, an `ORDER BY` column may refer to aliases of result columns
+  /// in the current statement: `SELECT foo AS bar FROM tbl ORDER BY bar` is
+  /// legal, but `SELECT foo AS bar FROM tbl WHERE bar < 10` is not. To control
+  /// whether result columns may be resolved, the [allowReferenceToResultColumn]
+  /// flag can be enabled.
+  ///
+  /// If an empty list is returned, the reference couldn't be resolved. If the
+  /// returned list contains more than one column, the lookup is ambigious.
   List<Column> resolveUnqualifiedReference(String columnName,
           {bool allowReferenceToResultColumn = false}) =>
       const [];
 }
 
+/// The root scope created by the SQL engine to analyze a statement.
+///
+/// This contains known tables (or views) and modules to look up.
 class RootScope extends ReferenceScope {
   @override
   RootScope get rootScope => this;
 
+  /// All tables (or views, or other result sets) that are known in the current
+  /// schema.
+  ///
+  /// [resolveResultSetToAdd] will query these tables by default.
   final Map<String, ResultSet> knownTables = CaseInsensitiveMap();
+
+  /// Known modules that are registered for this statement.
+  ///
+  /// This is used to resolve `CREATE VIRTUAL TABLE` statements.
   final Map<String, Module> knownModules = CaseInsensitiveMap();
 }
 
+/// A scope used by statements.
+///
+/// Tables added from `FROM` clauses are added to [resultSets], CTEs are added
+/// to [additionalKnownTables].
+///
+/// This is the scope most commonly used, but specific nodes may be attached to
+/// a different scope in case they have limited visibility. For instance,
+///  - foreign key clauses are wrapped in a [SingleTableReferenceScope] because
+///    they can't see unqualified columns of the overal scope.
+///  - subquery expressions can see parent tables and columns, but their columns
+///    aren't visible in the parent statement. This is implemented by wrapping
+///    them in a [StatementScope] as well.
+///  - subqueries appearing in a `FROM` clause _can't_ see outer columns and
+///    tables. These statements are also wrapped in a [StatementScope], but a
+///    [SubqueryInFromScope] is insertted as an intermediatet scope to prevent
+///    the inner scope from seeing the outer columns.
+
 class StatementScope extends ReferenceScope {
   final ReferenceScope parent;
 
+  /// Additional tables (that haven't necessarily been added in a `FROM` clause
+  /// that are only visible in this scope).
+  ///
+  /// This is commonly used for common table expressions, e.g a `WITH foo AS
+  /// (...)` would add a result set `foo` into the [additionalKnownTables] of
+  /// the overall statement, because `foo` can now be selected.
   final Map<String, ResultSet> additionalKnownTables = CaseInsensitiveMap();
+
+  /// Result sets that were added through a `FROM` clause and are now available
+  /// in this scope.
+  ///
+  /// The [ResultSetAvailableInStatement] contains information about the AST
+  /// node causing this statement to be available.
   final Map<String?, ResultSetAvailableInStatement> resultSets =
       CaseInsensitiveMap();
+
+  /// For select statements, additional columns available under a name because
+  /// there were added after the `SELECT`.
+  ///
+  /// This is used to resolve unqualified references by `ORDER BY` clauses.
   final List<Column> namedResultColumns = [];
 
   final Map<String, NamedWindowDeclaration> windowDeclarations =
@@ -88,8 +157,6 @@ class StatementScope extends ReferenceScope {
     final parent = this.parent;
     if (parent is StatementScope) {
       return parent;
-    } else if (parent is SubqueryInFromScope) {
-      return parent.enclosingStatement;
     } else if (parent is MiscStatementSubScope) {
       return parent.parent;
     } else {
@@ -200,6 +267,8 @@ 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 {
   final StatementScope enclosingStatement;
 
@@ -209,6 +278,11 @@ class SubqueryInFromScope extends ReferenceScope {
   RootScope get rootScope => enclosingStatement.rootScope;
 }
 
+/// A rarely used sub-scope for AST nodes that belong to a statement, but may
+/// have access to more result sets.
+///
+/// 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 {
   final StatementScope parent;
 

sqlparser/test/analysis/column_resolver_test.dart

@@ -2,6 +2,7 @@ import 'package:sqlparser/sqlparser.dart';
 import 'package:test/test.dart';
 
 import 'data.dart';
+import 'errors/utils.dart';
 
 void main() {
   late SqlEngine engine;
@@ -238,4 +239,18 @@ INSERT INTO demo VALUES (?, ?)
       expect(root.resolvedTargetColumns, hasLength(1));
     });
   });
+
+  test('does not allow a subquery in from to read outer values', () {
+    final result = engine.analyze(
+        'SELECT * FROM demo d1, (SELECT * FROM demo i WHERE i.id = d1.id) d2;');
+
+    result.expectError('d1.id', type: AnalysisErrorType.referencedUnknownTable);
+  });
+
+  test('allows subquery expressions to read outer values', () {
+    final result = engine.analyze('SELECT * FROM demo d1 WHERE '
+        'EXISTS (SELECT * FROM demo i WHERE i.id = d1.id);');
+
+    result.expectNoError();
+  });
 }