sqlparser: Extend support for `IN` expressions

This adds support for table names and table-valued function
invocations as right-hand targets in `IN` expressions.
The lint for row values around `IN` expressions is updated to
only report warnings if there's an actual mismatch in the amount
of columns.

Closes https://github.com/simolus3/drift/issues/2948

sqlparser/CHANGELOG.md

@@ -1,6 +1,8 @@
 ## 3.35.0-dev
 
 - Fix parsing binary literals.
+- Expand support for `IN` expressions, they now support tuples on the left-hand
+  side and the shorthand syntax for table references and table-valued functions.
 - Drift extensions: Allow custom class names for `CREATE VIEW` statements.
 
 ## 0.34.1

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

@@ -27,7 +27,7 @@ abstract class ReferenceScope {
   /// 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]).
+  /// scope. However, some exceptions apply (see e.g. [SourceScope]).
   Iterable<ResultSetAvailableInStatement> get resultSetAvailableToChildScopes =>
       const Iterable.empty();
 
@@ -167,8 +167,8 @@ mixin _HasParentScope on ReferenceScope {
 ///    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.
+///    [SourceScope] is inserted as an intermediate scope to prevent the inner
+///    scope from seeing the outer columns.
 
 class StatementScope extends ReferenceScope with _HasParentScope {
   final ReferenceScope parent;

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

@@ -170,6 +170,16 @@ class ColumnResolver extends RecursiveVisitor<ColumnResolverContext, void> {
     visitExcept(e, e.foreignTable, arg);
   }
 
+  @override
+  void visitInExpression(InExpression e, ColumnResolverContext arg) {
+    if (e.inside case Queryable query) {
+      _handle(query, [], arg);
+      visitExcept(e, e.inside, arg);
+    } else {
+      super.visitInExpression(e, arg);
+    }
+  }
+
   @override
   void visitUpdateStatement(UpdateStatement e, ColumnResolverContext arg) {
     // Resolve CTEs first

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

@@ -199,6 +199,56 @@ class LintingVisitor extends RecursiveVisitor<void, void> {
     visitChildren(e, arg);
   }
 
+  @override
+  void visitInExpression(InExpression e, void arg) {
+    final expectedColumns = switch (e.left) {
+      Tuple(:var expressions) => expressions.length,
+      _ => 1,
+    };
+
+    switch (e.inside) {
+      case Tuple tuple:
+        for (final element in tuple.expressions) {
+          final actualColumns = switch (element) {
+            Tuple(:var expressions) => expressions.length,
+            _ => 1,
+          };
+
+          if (expectedColumns != actualColumns) {
+            context.reportError(AnalysisError(
+              type: AnalysisErrorType.other,
+              message: 'Expected $expectedColumns columns in this entry, got '
+                  '$actualColumns',
+              relevantNode: element,
+            ));
+          }
+        }
+      case SubQuery subquery:
+        final columns = subquery.select.resolvedColumns;
+        if (columns != null && columns.length != expectedColumns) {
+          context.reportError(AnalysisError(
+            type: AnalysisErrorType.other,
+            message: 'The subquery must return $expectedColumns columns, '
+                'it returns ${columns.length}',
+            relevantNode: subquery,
+          ));
+        }
+      case TableOrSubquery table:
+        final columns =
+            table.availableResultSet?.resultSet.resultSet?.resolvedColumns;
+        if (columns != null && columns.length != expectedColumns) {
+          context.reportError(AnalysisError(
+            type: AnalysisErrorType.other,
+            message: 'To be used in this `IN` expression, this table must '
+                'have $expectedColumns columns (it has ${columns.length}).',
+            relevantNode: table,
+          ));
+        }
+    }
+
+    visitChildren(e, arg);
+  }
+
   @override
   void visitIsExpression(IsExpression e, void arg) {
     if (e.distinctFromSyntax && options.version < SqliteVersion.v3_39) {
@@ -526,9 +576,9 @@ class LintingVisitor extends RecursiveVisitor<void, void> {
         isAllowed = !comparisons.any((e) => !isRowValue(e));
       }
     } else if (parent is InExpression) {
-      // In expressions are tricky. The rhs can always be a row value, but the
-      // lhs can only be a row value if the rhs is a subquery
-      isAllowed = e == parent.inside || parent.inside is SubQuery;
+      // For in expressions we have a more accurate analysis on whether tuples
+      // are allowed that looks at both the LHS and the RHS.
+      isAllowed = true;
     } else if (parent is SetComponent) {
       isAllowed = true;
     }

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

@@ -136,6 +136,14 @@ class AstPreparingVisitor extends RecursiveVisitor<void, void> {
     visitChildren(e, arg);
   }
 
+  @override
+  void visitInExpression(InExpression e, void arg) {
+    // The RHS can use everything from the parent scope, but it can't add new
+    // table references that would be visible to others.
+    e.scope = StatementScope(e.scope);
+    visitChildren(e, arg);
+  }
+
   @override
   void visitNumberedVariable(NumberedVariable e, void arg) {
     _foundVariables.add(e);

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

@@ -427,9 +427,11 @@ class TypeResolver extends RecursiveVisitor<TypeExpectation, void> {
   @override
   void visitInExpression(InExpression e, TypeExpectation arg) {
     session._checkAndResolve(e, const ResolvedType.bool(), arg);
-    session._addRelation(NullableIfSomeOtherIs(e, e.childNodes));
 
-    session._addRelation(CopyTypeFrom(e.inside, e.left, array: true));
+    if (e.inside case Expression inExpr) {
+      session._addRelation(NullableIfSomeOtherIs(e, [e.left, inExpr]));
+      session._addRelation(CopyTypeFrom(inExpr, e.left, array: true));
+    }
 
     visitChildren(e, const NoTypeExpectation());
   }

sqlparser/lib/src/ast/common/queryables.dart

@@ -41,7 +41,7 @@ abstract class TableOrSubquery extends Queryable {
 /// set.
 class TableReference extends TableOrSubquery
     with ReferenceOwner
-    implements Renamable, ResolvesToResultSet {
+    implements Renamable, ResolvesToResultSet, InExpressionTarget {
   final String? schemaName;
   final String tableName;
   Token? tableNameToken;
@@ -213,7 +213,12 @@ class UsingConstraint extends JoinConstraint {
 }
 
 class TableValuedFunction extends Queryable
-    implements TableOrSubquery, SqlInvocation, Renamable, ResolvesToResultSet {
+    implements
+        TableOrSubquery,
+        SqlInvocation,
+        Renamable,
+        ResolvesToResultSet,
+        InExpressionTarget {
   @override
   final String name;
 

sqlparser/lib/src/ast/common/tuple.dart

@@ -3,7 +3,7 @@ part of '../ast.dart';
 /// A tuple of values, denotes in brackets. `(<expr>, ..., <expr>)`.
 ///
 /// In sqlite, this is also called a "row value".
-class Tuple extends Expression {
+class Tuple extends Expression implements InExpressionTarget {
   /// The expressions appearing in this tuple.
   List<Expression> expressions;
 

sqlparser/lib/src/ast/expressions/simple.dart

@@ -180,10 +180,9 @@ class InExpression extends Expression {
   /// against. From the sqlite grammar, we support [Tuple] and a [SubQuery].
   /// We also support a [Variable] as syntax sugar - it will be expanded into a
   /// tuple of variables at runtime.
-  Expression inside;
+  InExpressionTarget inside;
 
-  InExpression({this.not = false, required this.left, required this.inside})
-      : assert(inside is Tuple || inside is Variable || inside is SubQuery);
+  InExpression({this.not = false, required this.left, required this.inside});
 
   @override
   R accept<A, R>(AstVisitor<A, R> visitor, A arg) {
@@ -191,7 +190,7 @@ class InExpression extends Expression {
   }
 
   @override
-  List<Expression> get childNodes => [left, inside];
+  List<AstNode> get childNodes => [left, inside];
 
   @override
   void transformChildren<A>(Transformer<A> transformer, A arg) {
@@ -200,6 +199,19 @@ class InExpression extends Expression {
   }
 }
 
+/// Possible values for the right-hand side of an [InExpression].
+///
+/// Valid subclasses are:
+///  - [Tuple], to check whether the LHS is equal to any of the elements in the
+///    tuple.
+///  - [SubQuery], to check whether the LHS is equal to any of the rows returned
+///    by the subquery.
+///  - [TableReference] and [TableValuedFunction], a short-hand for [SubQuery]s
+///    if the table or function only return one column.
+///  - [Variable] (only if drift extensions are enabled), drift's generator
+///    turns this into a tuple of variables at runtime.
+abstract class InExpressionTarget implements AstNode {}
+
 class Parentheses extends Expression {
   Token? openingLeft;
   Token? closingRight;

sqlparser/lib/src/ast/expressions/subquery.dart

@@ -2,7 +2,7 @@ part of '../ast.dart';
 
 /// A subquery, which is an expression. It is expected that the inner query
 /// only returns one column and one row.
-class SubQuery extends Expression {
+class SubQuery extends Expression implements InExpressionTarget {
   BaseSelectStatement select;
 
   SubQuery({required this.select});

sqlparser/lib/src/ast/expressions/variables.dart

@@ -1,6 +1,6 @@
 part of '../ast.dart';
 
-abstract class Variable extends Expression {
+abstract class Variable extends Expression implements InExpressionTarget {
   int? resolvedIndex;
 }
 

sqlparser/lib/src/reader/parser.dart

@@ -511,7 +511,20 @@ class Parser {
       final not = _matchOne(TokenType.not);
       _matchOne(TokenType.$in);
 
-      final inside = _variableOrNull() ?? _consumeTuple(orSubQuery: true);
+      InExpressionTarget inside;
+      if (_variableOrNull() case var variable?) {
+        inside = variable;
+      } else if (_check(TokenType.leftParen)) {
+        inside = _consumeTuple(orSubQuery: true) as InExpressionTarget;
+      } else {
+        final target = _tableOrSubquery();
+        // TableOrSubquery is either a table reference, a table-valued function,
+        // or a Subquery. We don't support subqueries, but they can't be parsed
+        // here because we would have entered the tuple case above.
+        assert(target is! SubQuery);
+        inside = target as InExpressionTarget;
+      }
+
       return InExpression(left: left, inside: inside, not: not)
         ..setSpan(left.first!, _previous);
     }

sqlparser/test/analysis/column_resolver_test.dart

@@ -363,4 +363,10 @@ SELECT * FROM cars
 
     expect(select.resolvedColumns?.map((e) => e.name), ['literal', 'bar']);
   });
+
+  test('error for nonexisting table in IN expression', () {
+    final query = engine.analyze('SELECT 1 IN no_such_table');
+    query.expectError('no_such_table',
+        type: AnalysisErrorType.referencedUnknownTable);
+  });
 }

sqlparser/test/analysis/errors/row_value_misuse_test.dart

@@ -6,7 +6,8 @@ import 'utils.dart';
 void main() {
   late SqlEngine engine;
   setUp(() {
-    engine = SqlEngine();
+    // enable json1 extension
+    engine = SqlEngine(EngineOptions(version: SqliteVersion.v3_38));
   });
 
   test('when using row value in select', () {
@@ -15,12 +16,6 @@ void main() {
         .expectError('(1, 2, 3)', type: AnalysisErrorType.rowValueMisuse);
   });
 
-  test('as left hand operator of in', () {
-    engine
-        .analyze('SELECT (1, 2, 3) IN (4, 5, 6)')
-        .expectError('(1, 2, 3)', type: AnalysisErrorType.rowValueMisuse);
-  });
-
   test('in BETWEEN expression', () {
     engine
         .analyze('SELECT 1 BETWEEN (1, 2, 3) AND 3')
@@ -68,4 +63,33 @@ void main() {
           .expectNoError();
     });
   });
+
+  group('in expressions', () {
+    test('when tuple is expected', () {
+      engine.analyze('SELECT (1, 2) IN ((4, 5), 6)').expectError('6',
+          type: AnalysisErrorType.other,
+          message: contains('Expected 2 columns in this entry, got 1'));
+    });
+
+    test('when tuple is not expected', () {
+      engine.analyze('SELECT 1 IN ((4, 5), 6)').expectError('(4, 5)',
+          type: AnalysisErrorType.other,
+          message: contains('Expected 1 columns in this entry, got 2'));
+    });
+
+    test('for table reference', () {
+      engine
+          .analyze('WITH names AS (VALUES(1, 2, 3)) SELECT (1, 2) IN names')
+          .expectError('names',
+              type: AnalysisErrorType.other,
+              message: contains('must have 2 columns (it has 3)'));
+    });
+
+    test('for table-valued function', () {
+      engine.analyze("SELECT (1, 2) IN json_each('{}')").expectError(
+          "json_each('{}')",
+          type: AnalysisErrorType.other,
+          message: contains('this table must have 2 columns (it has 8)'));
+    });
+  });
 }

sqlparser/test/analysis/errors/utils.dart

@@ -2,10 +2,10 @@ import 'package:sqlparser/sqlparser.dart';
 import 'package:test/test.dart';
 
 extension ExpectErrors on AnalysisContext {
-  void expectError(String lexeme, {AnalysisErrorType? type}) {
+  void expectError(String lexeme, {AnalysisErrorType? type, message}) {
     expect(
       errors,
-      [analysisErrorWith(lexeme: lexeme, type: type)],
+      [analysisErrorWith(lexeme: lexeme, type: type, message: message)],
     );
   }
 

sqlparser/test/parser/expression_test.dart

@@ -160,6 +160,22 @@ final Map<String, Expression> _testCases = {
       ],
     ),
   ),
+  'x IN json_each(bar)': InExpression(
+    left: Reference(columnName: 'x'),
+    inside: TableValuedFunction(
+      'json_each',
+      ExprFunctionParameters(
+        parameters: [
+          Reference(columnName: 'bar'),
+        ],
+      ),
+    ),
+  ),
+  'x NOT IN "table"': InExpression(
+    not: true,
+    left: Reference(columnName: 'x'),
+    inside: TableReference('table'),
+  ),
   'CAST(3 + 4 AS TEXT)': CastExpression(
     BinaryExpression(
       NumericLiteral(3.0),

sqlparser/test/utils/node_to_text_test.dart

@@ -485,6 +485,9 @@ CREATE UNIQUE INDEX my_idx ON t1 (c1, c2, c3) WHERE c1 < c3;
     test('in', () {
       testFormat('SELECT x IN (SELECT * FROM foo);');
       testFormat('SELECT x NOT IN (SELECT * FROM foo);');
+      testFormat('SELECT x IN foo');
+      testFormat('SELECT x IN json_each(bar)');
+      testFormat('SELECT x IN :array');
     });
 
     test('boolean literals', () {