Smarter many-to-one propagation in types2

moor/build.yaml

@@ -7,7 +7,7 @@ targets:
           use_column_name_as_json_key_when_defined_in_moor_file: true
           generate_connect_constructor: true
           write_from_json_string_constructor: true
-          use_experimental_inference: true
+#          use_experimental_inference: true
           sqlite_modules:
             - json1
             - fts5

moor_generator/test/analyzer/experimental_inference_test.dart

@@ -0,0 +1,57 @@
+import 'package:moor_generator/moor_generator.dart';
+import 'package:moor_generator/src/analyzer/options.dart';
+import 'package:moor_generator/src/analyzer/runner/results.dart';
+import 'package:test/test.dart';
+
+import 'utils.dart';
+
+void main() {
+  test('experimental inference - integration test', () async {
+    final state = TestState.withContent({
+      'foo|lib/a.moor': '''
+CREATE TABLE artists (
+  id INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT,
+  name VARCHAR NOT NULL
+);
+
+CREATE TABLE albums (
+  id INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT,
+  name TEXT NOT NULL,
+  artist INTEGER NOT NULL REFERENCES artists (id)
+);
+
+CREATE TABLE tracks (
+  id INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT,
+  name TEXT NOT NULL,
+  album INTEGER NOT NULL REFERENCES albums (id),
+  duration_seconds INTEGER NOT NULL,
+  was_single BOOLEAN NOT NULL DEFAULT FALSE
+);
+
+totalDurationByArtist:
+  SELECT a.*, SUM(tracks.duration_seconds) AS duration
+  FROM artists a
+    INNER JOIN albums ON albums.artist = a.id
+    INNER JOIN tracks ON tracks.album = albums.id
+  GROUP BY artists.id;
+    '''
+    }, options: const MoorOptions(useExperimentalInference: true));
+
+    final file = await state.analyze('package:foo/a.moor');
+    final result = file.currentResult as ParsedMoorFile;
+    final queries = result.resolvedQueries;
+
+    expect(state.session.errorsInFileAndImports(file), isEmpty);
+
+    final totalDurationByArtist =
+        queries.singleWhere((q) => q.name == 'totalDurationByArtist');
+    expect(
+      totalDurationByArtist,
+      returnsColumns({
+        'id': ColumnType.integer,
+        'name': ColumnType.text,
+        'duration': ColumnType.integer,
+      }),
+    );
+  });
+}

moor_generator/test/analyzer/utils.dart

@@ -1,7 +1,10 @@
 import 'package:build/build.dart';
+import 'package:moor_generator/moor_generator.dart';
+import 'package:moor_generator/src/analyzer/options.dart';
 import 'package:moor_generator/src/analyzer/runner/file_graph.dart';
 import 'package:moor_generator/src/analyzer/runner/task.dart';
 import 'package:moor_generator/src/analyzer/session.dart';
+import 'package:test/test.dart';
 
 import '../utils/test_backend.dart';
 
@@ -11,12 +14,16 @@ class TestState {
 
   TestState(this.backend, this.session);
 
-  factory TestState.withContent(Map<String, String> content) {
+  factory TestState.withContent(Map<String, String> content,
+      {MoorOptions options}) {
     final backend = TestBackend({
       for (final entry in content.entries)
         AssetId.parse(entry.key): entry.value,
     });
     final session = MoorSession(backend);
+    if (options != null) {
+      session.options = options;
+    }
     return TestState(backend, session);
   }
 
@@ -36,3 +43,24 @@ class TestState {
     return file(uri);
   }
 }
+
+// Matchers
+Matcher returnsColumns(Map<String, ColumnType> columns) {
+  return _HasInferredColumnTypes(columns);
+}
+
+class _HasInferredColumnTypes extends CustomMatcher {
+  _HasInferredColumnTypes(dynamic expected)
+      : super('Select query with inferred columns', 'columns', expected);
+
+  @override
+  Object featureValueOf(dynamic actual) {
+    if (actual is! SqlSelectQuery) {
+      return actual;
+    }
+
+    final query = actual as SqlSelectQuery;
+    final resultSet = query.resultSet;
+    return {for (final column in resultSet.columns) column.name: column.type};
+  }
+}

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

@@ -1,16 +1,20 @@
 part of '../types.dart';
 
-/// Dependency declaring that [target] is nullable if any in [other] is
+/// Dependency declaring that [target] is nullable if any in [from] is
 /// nullable.
-class NullableIfSomeOtherIs extends TypeRelationship {
+class NullableIfSomeOtherIs extends TypeRelationship
+    implements MultiSourceRelation {
+  @override
   final Typeable target;
-  final List<Typeable> other;
+  @override
+  final List<Typeable> from;
 
-  NullableIfSomeOtherIs(this.target, this.other);
+  NullableIfSomeOtherIs(this.target, this.from);
 }
 
 /// Dependency declaring that [target] has exactly the same type as [other].
-class CopyTypeFrom extends TypeRelationship {
+class CopyTypeFrom extends TypeRelationship implements DirectedRelation {
+  @override
   final Typeable target;
   final Typeable other;
 
@@ -18,8 +22,11 @@ class CopyTypeFrom extends TypeRelationship {
 }
 
 /// Dependency declaring that [target] has a type that matches all of [from].
-class CopyEncapsulating extends TypeRelationship {
+class CopyEncapsulating extends TypeRelationship
+    implements MultiSourceRelation {
+  @override
   final Typeable target;
+  @override
   final List<Typeable> from;
 
   CopyEncapsulating(this.target, this.from);
@@ -42,7 +49,8 @@ class HaveSameType extends TypeRelationship {
 
 /// Dependency declaring that, if no better option is found, [target] should
 /// have the specified [defaultType].
-class DefaultType extends TypeRelationship {
+class DefaultType extends TypeRelationship implements DirectedRelation {
+  @override
   final Typeable target;
   final ResolvedType defaultType;
 
@@ -53,7 +61,8 @@ enum CastMode { numeric, boolean }
 
 /// Dependency declaring that [target] has the same type as [other] after
 /// casting it with [cast].
-class CopyAndCast extends TypeRelationship {
+class CopyAndCast extends TypeRelationship implements DirectedRelation {
+  @override
   final Typeable target;
   final Typeable other;
   final CastMode cast;

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

@@ -10,7 +10,6 @@ class TypeGraph {
 
   final Map<Typeable, List<TypeRelationship>> _edges = {};
   final List<DefaultType> _defaultTypes = [];
-  final List<CopyEncapsulating> _manyToOne = [];
 
   TypeGraph();
 
@@ -48,17 +47,6 @@ class TypeGraph {
       _propagateTypeInfo(queue, queue.removeLast());
     }
 
-    // propagate many-to-one changes
-    for (final edge in _manyToOne) {
-      if (!knowsType(edge.target)) {
-        final fromTypes = edge.from.map((t) => this[t]).where((e) => e != null);
-        final encapsulated = _encapsulate(fromTypes);
-        if (encapsulated != null) {
-          this[edge.target] = encapsulated;
-        }
-      }
-    }
-
     // apply default types
     for (final applyDefault in _defaultTypes) {
       if (!knowsType(applyDefault.target)) {
@@ -70,21 +58,43 @@ class TypeGraph {
   void _propagateTypeInfo(List<Typeable> resolved, Typeable t) {
     if (!_edges.containsKey(t)) return;
 
-    // propagate one-to-one and one-to-many changes
+    // propagate changes
     for (final edge in _edges[t]) {
       if (edge is CopyTypeFrom) {
         _copyType(resolved, edge.other, edge.target);
       } else if (edge is HaveSameType) {
         _copyType(resolved, t, edge.getOther(t));
+      } else if (edge is CopyAndCast) {
+        _copyType(resolved, t, edge.target, this[t].cast(edge.cast));
+      } else if (edge is MultiSourceRelation) {
+        // handle many-to-one changes, if all targets have been resolved
+        final typedEdge = edge as MultiSourceRelation;
+
+        if (typedEdge.from.every(knowsType)) {
+          _propagateManyToOne(typedEdge, resolved, t);
+        }
       }
     }
   }
 
-  void _copyType(List<Typeable> resolved, Typeable from, Typeable to) {
+  void _propagateManyToOne(
+      MultiSourceRelation edge, List<Typeable> resolved, Typeable t) {
+    if (!knowsType(edge.target)) {
+      final fromTypes = edge.from.map((t) => this[t]).where((e) => e != null);
+      final encapsulated = _encapsulate(fromTypes);
+      if (encapsulated != null) {
+        this[edge.target] = encapsulated;
+        resolved.add(edge.target);
+      }
+    }
+  }
+
+  void _copyType(List<Typeable> resolved, Typeable from, Typeable to,
+      [ResolvedType type]) {
     // if the target hasn't been resolved yet, copy the current type and
     // visit the target later
     if (!knowsType(to)) {
-      this[to] = this[from];
+      this[to] = type ?? this[from];
       resolved.add(to);
     }
   }
@@ -110,11 +120,11 @@ class TypeGraph {
 
     for (final relation in _relationships) {
       if (relation is NullableIfSomeOtherIs) {
-        putAll(relation.other, relation);
+        putAll(relation.from, relation);
       } else if (relation is CopyTypeFrom) {
         put(relation.other, relation);
       } else if (relation is CopyEncapsulating) {
-        _manyToOne.add(relation);
+        putAll(relation.from, relation);
       } else if (relation is HaveSameType) {
         put(relation.first, relation);
         put(relation.second, relation);
@@ -131,6 +141,14 @@ class TypeGraph {
 
 abstract class TypeRelationship {}
 
+abstract class DirectedRelation {
+  Typeable get target;
+}
+
+abstract class MultiSourceRelation implements DirectedRelation {
+  List<Typeable> get from;
+}
+
 /// Keeps track of resolved variable types so that they can be re-used.
 /// Different [Variable] instances can refer to the same logical sql variable,
 /// so we keep track of them.
@@ -158,4 +176,17 @@ extension on ResolvedType {
     // fallback. todo: Support more cases
     return const ResolvedType(type: BasicType.text, nullable: true);
   }
+
+  ResolvedType cast(CastMode mode) {
+    switch (mode) {
+      case CastMode.numeric:
+        if (type == BasicType.int || type == BasicType.real) return this;
+
+        return const ResolvedType(type: BasicType.real);
+      case CastMode.boolean:
+        return const ResolvedType.bool();
+    }
+
+    throw AssertionError('all switch statements handled');
+  }
 }

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

@@ -15,6 +15,11 @@ class TypeResolver extends RecursiveVisitor<TypeExpectation, void> {
 
   void run(AstNode root) {
     visit(root, const NoTypeExpectation());
+
+    // annotate Columns as well. They implement Typeable, but aren't an ast
+    // node, which means this visitor doesn't find them
+    root.acceptWithoutArg(_ResultColumnVisitor(this));
+
     session.finish();
   }
 
@@ -138,8 +143,24 @@ class TypeResolver extends RecursiveVisitor<TypeExpectation, void> {
 
   @override
   void visitVariable(Variable e, TypeExpectation arg) {
-    final resolved = session.context.stmtOptions.specifiedTypeOf(e) ??
+    _inferAsVariable(e, arg);
-        _inferFromContext(arg);
+  }
+
+  @override
+  void visitDartPlaceholder(DartPlaceholder e, TypeExpectation arg) {
+    if (e is DartExpressionPlaceholder) {
+      _inferAsVariable(e, arg);
+    } else {
+      super.visitDartPlaceholder(e, arg);
+    }
+  }
+
+  void _inferAsVariable(Expression e, TypeExpectation arg) {
+    ResolvedType resolved;
+    if (e is Variable) {
+      resolved = session.context.stmtOptions.specifiedTypeOf(e);
+    }
+    resolved ??= _inferFromContext(arg);
 
     if (resolved != null) {
       session.checkAndResolve(e, resolved, arg);
@@ -511,3 +532,16 @@ class TypeResolver extends RecursiveVisitor<TypeExpectation, void> {
     return null;
   }
 }
+
+class _ResultColumnVisitor extends RecursiveVisitor<void, void> {
+  final TypeResolver resolver;
+
+  _ResultColumnVisitor(this.resolver);
+
+  @override
+  void visitBaseSelectStatement(BaseSelectStatement stmt, void arg) {
+    if (stmt.resolvedColumns != null) {
+      stmt.resolvedColumns.forEach(resolver._handleColumn);
+    }
+  }
+}

sqlparser/lib/src/engine/sql_engine.dart

@@ -203,8 +203,7 @@ class SqlEngine {
       if (options.enableExperimentalTypeInference) {
         final session = t2.TypeInferenceSession(context, options);
         final resolver = t2.TypeResolver(session);
-        node.acceptWithoutArg(resolver);
+        resolver.run(node);
-        session.finish();
         context.types2 = session.results;
       } else {
         node.acceptWithoutArg(TypeResolvingVisitor(context));

sqlparser/test/analysis/types2/resolver_test.dart

@@ -5,7 +5,7 @@ import 'package:test/test.dart';
 import '../data.dart';
 
 void main() {
-  final engine = SqlEngine()
+  final engine = SqlEngine.withOptions(EngineOptions(useMoorExtensions: true))
     ..registerTable(demoTable)
     ..registerTable(anotherTable);
 
@@ -27,8 +27,7 @@ void main() {
     final resolver = _obtainResolver(sql);
     final session = resolver.session;
     final stmt = session.context.root as SelectStatement;
-    return session
+    return session.typeOf(stmt.resolvedColumns.single);
-        .typeOf((stmt.columns.single as ExpressionResultColumn).expression);
   }
 
   test('resolves literals', () {
@@ -165,6 +164,21 @@ void main() {
       expect(type, const ResolvedType(type: BasicType.text));
     });
 
+    test('can select columns', () {
+      final type = _resolveResultColumn('SELECT id FROM demo;');
+      expect(type, const ResolvedType(type: BasicType.int));
+    });
+
+    test('infers types for dart placeholders', () {
+      final resolver = _obtainResolver(r'SELECT * FROM demo WHERE $pred');
+      final type = resolver.session.typeOf(resolver
+              .session.context.root.allDescendants
+              .firstWhere((e) => e is DartExpressionPlaceholder)
+          as DartExpressionPlaceholder);
+
+      expect(type, const ResolvedType.bool());
+    });
+
     test('handles recursive CTEs', () {
       final type = _resolveResultColumn('''
 WITH RECURSIVE