Make columns from LEFT OUTER joins nullable

moor/pubspec.yaml

@@ -19,7 +19,6 @@ dependencies:
   sqlite3: ^0.1.9-nullsafety
 
 dev_dependencies:
-  analyzer: ^0.40.6
   build_test: ^1.3.0
   build_runner_core: ^6.1.0
   moor_generator: ^3.2.0
@@ -40,7 +39,7 @@ dependency_overrides:
   mockito:
     git:
       url: https://github.com/simolus3/mockito.git
-      ref: generate-inheritance
+      ref: version-for-moor
 
   uuid:
     git:

moor/test/data/utils/mocks.mocks.dart

@@ -5,6 +5,8 @@ import 'dart:async' as _i4;
 import 'package:moor/src/runtime/executor/stream_queries.dart' as _i5;
 import 'package:moor/src/runtime/api/runtime_api.dart' as _i6;
 
+class _FakeType extends _i1.Fake implements Type {}
+
 class _FakeTransactionExecutor extends _i1.Fake
     implements _i2.TransactionExecutor {}
 
@@ -18,6 +20,9 @@ class MockExecutorInternal extends _i1.Mock implements _i2.QueryExecutor {
 
   _i3.SqlDialect get dialect =>
       super.noSuchMethod(Invocation.getter(#dialect), _i3.SqlDialect.sqlite);
+  int get hashCode => super.noSuchMethod(Invocation.getter(#hashCode), 0);
+  Type get runtimeType =>
+      super.noSuchMethod(Invocation.getter(#runtimeType), _FakeType());
   _i4.Future<bool> ensureOpen(_i2.QueryExecutorUser? user) =>
       super.noSuchMethod(
           Invocation.method(#ensureOpen, [user]), Future.value(false));
@@ -44,6 +49,9 @@ class MockExecutorInternal extends _i1.Mock implements _i2.QueryExecutor {
       Invocation.method(#beginTransaction, []), _FakeTransactionExecutor());
   _i4.Future<void> close() =>
       super.noSuchMethod(Invocation.method(#close, []), Future.value(null));
+  bool operator ==(Object? other) =>
+      super.noSuchMethod(Invocation.method(#==, [other]), false);
+  String toString() => super.noSuchMethod(Invocation.method(#toString, []), '');
 }
 
 /// A class which mocks [TransactionExecutor].
@@ -55,6 +63,11 @@ class MockTransactionsInternal extends _i1.Mock
     _i1.throwOnMissingStub(this);
   }
 
+  _i3.SqlDialect get dialect =>
+      super.noSuchMethod(Invocation.getter(#dialect), _i3.SqlDialect.sqlite);
+  int get hashCode => super.noSuchMethod(Invocation.getter(#hashCode), 0);
+  Type get runtimeType =>
+      super.noSuchMethod(Invocation.getter(#runtimeType), _FakeType());
   _i4.Future<void> send() =>
       super.noSuchMethod(Invocation.method(#send, []), Future.value(null));
   _i4.Future<void> rollback() =>
@@ -85,12 +98,18 @@ class MockTransactionsInternal extends _i1.Mock
       Invocation.method(#beginTransaction, []), _FakeTransactionExecutor());
   _i4.Future<void> close() =>
       super.noSuchMethod(Invocation.method(#close, []), Future.value(null));
+  bool operator ==(Object? other) =>
+      super.noSuchMethod(Invocation.method(#==, [other]), false);
+  String toString() => super.noSuchMethod(Invocation.method(#toString, []), '');
 }
 
 /// A class which mocks [StreamQueryStore].
 ///
 /// See the documentation for Mockito's code generation for more information.
 class MockStreamQueries extends _i1.Mock implements _i5.StreamQueryStore {
+  int get hashCode => super.noSuchMethod(Invocation.getter(#hashCode), 0);
+  Type get runtimeType =>
+      super.noSuchMethod(Invocation.getter(#runtimeType), _FakeType());
   _i4.Stream<T> registerStream<T>(_i5.QueryStreamFetcher<T>? fetcher) =>
       super.noSuchMethod(
           Invocation.method(#registerStream, [fetcher]), Stream<T>.empty());
@@ -107,4 +126,7 @@ class MockStreamQueries extends _i1.Mock implements _i5.StreamQueryStore {
       super.noSuchMethod(Invocation.method(#markAsOpened, [stream]));
   _i4.Future<void> close() =>
       super.noSuchMethod(Invocation.method(#close, []), Future.value(null));
+  bool operator ==(Object? other) =>
+      super.noSuchMethod(Invocation.method(#==, [other]), false);
+  String toString() => super.noSuchMethod(Invocation.method(#toString, []), '');
 }

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

@@ -13,6 +13,10 @@ abstract class Column
   /// Some columns, notably the rowid aliases, are exempt from this.
   bool get includedInResults => true;
 
+  /// The result set containing this column, or null if this column is not part
+  /// of a known result set.
+  ResultSet? containingSet;
+
   @override
   String humanReadableDescription() {
     return name;
@@ -31,6 +35,9 @@ class TableColumn extends Column implements ColumnWithType {
   @override
   final String name;
 
+  @override
+  ResultSet? get containingSet => table;
+
   @override
   ResolvedType get type => _type;
   ResolvedType _type;
@@ -119,6 +126,9 @@ class ViewColumn extends Column with DelegatedColumn implements ColumnWithType {
   @override
   final Column innerColumn;
 
+  @override
+  ResultSet? get containingSet => view;
+
   /// The view this column belongs to.
   View? view;
 

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

@@ -268,7 +268,8 @@ class ColumnResolver extends RecursiveVisitor<void, void> {
           if (set!.length > i) set[i]
       ];
 
-      resolved.add(CompoundSelectColumn(columnsAtThisIndex));
+      resolved.add(
+          CompoundSelectColumn(columnsAtThisIndex)..containingSet = statement);
     }
     statement.resolvedColumns = resolved;
   }
@@ -293,7 +294,8 @@ class ColumnResolver extends RecursiveVisitor<void, void> {
           .map((tuple) => tuple.expressions[i])
           .toList();
 
-      columns.add(ValuesSelectColumn(columnName, expressions));
+      columns.add(ValuesSelectColumn(columnName, expressions)
+        ..containingSet = statement);
     }
 
     statement.resolvedColumns = columns;

sqlparser/lib/src/analysis/types/graph/relationships.dart

@@ -23,7 +23,11 @@ class CopyTypeFrom extends TypeRelation implements DirectedRelation {
   /// transformed.
   final bool? array;
 
-  CopyTypeFrom(this.target, this.other, {this.array});
+  /// Whether [target] is the nullable version of [other].
+  final bool makeNullable;
+
+  CopyTypeFrom(this.target, this.other,
+      {this.array, this.makeNullable = false});
 }
 
 /// Dependency declaring that [target] has a type that matches all of [from].

sqlparser/lib/src/analysis/types/graph/type_graph.dart

@@ -92,6 +92,9 @@ class TypeGraph {
         if (edge.array != null) {
           type = type!.toArray(edge.array);
         }
+        if (edge.makeNullable) {
+          _knownNullability[edge.target] = true;
+        }
         _copyType(resolved, edge.other, edge.target, type);
       } else if (edge is HaveSameType) {
         _copyType(resolved, t, edge.getOther(t));

sqlparser/lib/src/analysis/types/join_analysis.dart

@@ -0,0 +1,101 @@
+import 'package:sqlparser/src/analysis/analysis.dart';
+import 'package:sqlparser/src/ast/ast.dart';
+
+/// Tracks table references that must be non-nullable in a query row.
+///
+/// This is used to determine the nullability of column references. For
+/// instance, consider the following query:
+///
+/// ```sql
+/// SELECT foo.x, bar.y FROM foo
+///   LEFT OUTER JOIN bar ON ...
+/// ```
+///
+/// Clearly, `foo.x` is non-nullable in the result if `x` is a non-nullable
+/// column in `foo`. We can't say the same thing about `bar.y` though: Even if
+/// the column is declared to be `NOT NULL`, `bar` might be nullable in this
+/// query.
+///
+/// A [JoinModel] is attached to each basic [SelectStatement]. You can obtain
+/// the model for any ast node via [JoinModel.of]. It will lookup the model from
+/// the enclosing [SelectStatement], if there is one.
+///
+/// If a [Reference] refers to a table that's in [JoinModel.nonNullable] and
+/// the resolved column is non-nullable, we can assume that the reference is
+/// going to be non-nullable too.
+///
+/// At the moment, we consider the following tables to be
+/// [JoinModel.nonNullable]:
+///  - the "primary" table of a select statement (`foo` in the example above)
+///  - inner, cross, regular (no keyword, comma) joins
+///
+/// In the future, we'll also consider foreign key constraints.
+class JoinModel {
+  final List<ResolvesToResultSet> nonNullable;
+
+  JoinModel._(this.nonNullable);
+
+  factory JoinModel._resolve(SelectStatement statement) {
+    final visitor = _FindNonNullableJoins();
+    visitor.visitSelectStatement(statement, true);
+
+    return JoinModel._(visitor.nonNullable);
+  }
+
+  static JoinModel? of(AstNode node) {
+    final enclosingSelect = node.enclosingOfType<SelectStatement>();
+    if (enclosingSelect == null) return null;
+
+    final existing = enclosingSelect.meta<JoinModel>();
+    if (existing != null) return existing;
+
+    final created = JoinModel._resolve(enclosingSelect);
+    enclosingSelect.setMeta(created);
+    return created;
+  }
+
+  /// Checks whether the column comes from a nullable table.
+  bool isFromNullableTable(Column column) {
+    final resultSet = column.containingSet;
+    if (resultSet == null) return false;
+
+    return nonNullable.every((nonNullableRef) {
+      return nonNullableRef.resultSet != column.containingSet;
+    });
+  }
+}
+
+class _FindNonNullableJoins extends RecursiveVisitor<bool, void> {
+  final List<ResolvesToResultSet> nonNullable = [];
+
+  // The boolean arg indicates whether a visited queryable is needed for the
+  // result to have any rows (which, in particular, mean's its non-nullable)
+
+  @override
+  void visitSelectStatement(SelectStatement e, bool arg) {
+    visitNullable(e.from, true);
+  }
+
+  @override
+  void visitJoinClause(JoinClause e, bool arg) {
+    if (!arg) return;
+
+    visit(e.primary, true);
+    for (final additional in e.joins) {
+      if (additional.operator != JoinOperator.left &&
+          additional.operator != JoinOperator.leftOuter) {
+        visit(additional, true);
+      }
+    }
+  }
+
+  @override
+  void visitTableReference(TableReference e, bool arg) {
+    if (arg) nonNullable.add(e);
+  }
+
+  @override
+  void visitSelectStatementAsSource(SelectStatementAsSource e, bool arg) {
+    if (arg) nonNullable.add(e.statement);
+  }
+}

sqlparser/lib/src/analysis/types/resolving_visitor.dart

@@ -430,7 +430,9 @@ class TypeResolver extends RecursiveVisitor<TypeExpectation, void> {
     if (resolved == null) return;
 
     _handleColumn(resolved);
-    _lazyCopy(e, resolved);
+
+    final isNullable = JoinModel.of(e)?.isFromNullableTable(resolved) ?? false;
+    _lazyCopy(e, resolved, makeNullable: isNullable);
   }
 
   @override
@@ -633,11 +635,13 @@ class TypeResolver extends RecursiveVisitor<TypeExpectation, void> {
     }
   }
 
-  void _lazyCopy(Typeable to, Typeable? from) {
+  void _lazyCopy(Typeable to, Typeable? from, {bool makeNullable = false}) {
     if (session.graph.knowsType(from)) {
-      session._markTypeResolved(to, session.typeOf(from)!);
+      var type = session.typeOf(from)!;
+      if (makeNullable) type = type.withNullable(true);
+      session._markTypeResolved(to, type);
     } else {
-      session._addRelation(CopyTypeFrom(to, from));
+      session._addRelation(CopyTypeFrom(to, from, makeNullable: makeNullable));
     }
   }
 

sqlparser/lib/src/analysis/types/types.dart

@@ -1,10 +1,10 @@
 import 'package:sqlparser/sqlparser.dart';
 
-part 'expectation.dart';
+import 'join_analysis.dart';
 
+part 'expectation.dart';
 part 'graph/relationships.dart';
 part 'graph/type_graph.dart';
-
 part 'resolving_visitor.dart';
 
 /// Contains all information associated to a single type inference pass.

sqlparser/lib/src/ast/clauses/with.dart

@@ -62,8 +62,9 @@ class CommonTableExpression extends AstNode with ResultSet {
     // the select statement
     if (columnNames == null) return columnsOfSelect;
 
-    final cached = _cachedColumns ??=
-        columnNames!.map((name) => CommonTableExpressionColumn(name)).toList();
+    final cached = _cachedColumns ??= columnNames!
+        .map((name) => CommonTableExpressionColumn(name)..containingSet = this)
+        .toList();
 
     if (columnsOfSelect != null) {
       // bind the CommonTableExpressionColumn to the real underlying column

sqlparser/test/analysis/types2/resolver_test.dart

@@ -270,4 +270,28 @@ WITH RECURSIVE
       expect(type, const ResolvedType(type: BasicType.int, isArray: false));
     });
   });
+
+  test('columns from LEFT OUTER joins are nullable', () {
+    final resolver = _obtainResolver('''
+    WITH
+     sq_1 (one ) AS (SELECT 1),
+     sq_2 (two) AS (SELECT 2),
+     sq_3 (three) AS (SELECT 3)
+    
+    SELECT one, two, three
+     FROM sq_1
+     LEFT JOIN sq_2
+     LEFT OUTER JOIN sq_3
+    ''');
+
+    final session = resolver.session;
+    final stmt = resolver.session.context.root as SelectStatement;
+    final columns = stmt.resolvedColumns!;
+
+    expect(session.typeOf(columns[0]), const ResolvedType(type: BasicType.int));
+    expect(session.typeOf(columns[1]),
+        const ResolvedType(type: BasicType.int, nullable: true));
+    expect(session.typeOf(columns[2]),
+        const ResolvedType(type: BasicType.int, nullable: true));
+  });
 }