Desugar duplicate variables

drift/lib/src/runtime/query_builder/generation_context.dart

@@ -58,6 +58,15 @@ class GenerationContext {
   /// Gets the generated sql statement
   String get sql => buffer.toString();
 
+  /// The variable indices occupied by this generation context.
+  ///
+  /// SQL variables are 1-indexed, so a context with three variables would
+  /// cover the variables `1`, `2` and `3` by default.
+  Iterable<int> get variableIndices {
+    final start = explicitVariableIndex ?? 1;
+    return Iterable.generate(amountOfVariables, (i) => start + i);
+  }
+
   /// Constructs a [GenerationContext] by copying the relevant fields from the
   /// database.
   GenerationContext.fromDb(DatabaseConnectionUser this.executor,

drift/lib/src/runtime/query_builder/query_builder.dart

@@ -168,4 +168,24 @@ enum SqlDialect {
     this.escapeChar = '"',
     this.supportsIndexedParameters = true,
   });
+
+  /// For dialects that don't support named or explicitly-indexed variables,
+  /// translates a variable assignment to avoid using that feature.
+  ///
+  /// For instance, the SQL snippet `WHERE x = :a OR y = :a` would be translated
+  /// to `WHERE x = ? OR y = ?`. Then, [original] would contain the value for
+  /// the single variable and [syntacticOccurences] would contain two values
+  /// (`1` and `1`) referencing the original variable.
+  List<Variable> desugarDuplicateVariables(
+    List<Variable> original,
+    List<int> syntacticOccurences,
+  ) {
+    if (supportsIndexedParameters) return original;
+
+    return [
+      for (final occurence in syntacticOccurences)
+        // Variables in SQL are 1-indexed
+        original[occurence - 1],
+    ];
+  }
 }

drift_dev/lib/src/analysis/resolver/queries/query_analyzer.dart

@@ -22,6 +22,7 @@ import 'required_variables.dart';
 /// class is simply there to bundle the data.
 class _QueryHandlerContext {
   final List<FoundElement> foundElements;
+  final List<SyntacticElementReference> elementReferences;
   final AstNode root;
   final NestedQueriesContainer? nestedScope;
   final String queryName;
@@ -32,13 +33,15 @@ class _QueryHandlerContext {
 
   _QueryHandlerContext({
     required List<FoundElement> foundElements,
+    required List<SyntacticElementReference> elementReferences,
     required this.root,
     required this.queryName,
     required this.nestedScope,
     this.requestedResultClass,
     this.requestedResultType,
     this.sourceForFixedName,
-  }) : foundElements = List.unmodifiable(foundElements);
+  })  : foundElements = List.unmodifiable(foundElements),
+        elementReferences = List.unmodifiable(elementReferences);
 }
 
 /// Maps an [AnalysisContext] from the sqlparser to a [SqlQuery] from this
@@ -103,7 +106,7 @@ class QueryAnalyzer {
       nestedScope = nestedAnalyzer.analyzeRoot(context.root as SelectStatement);
     }
 
-    final foundElements = _extractElements(
+    final (foundElements, references) = _extractElements(
       ctx: context,
       root: context.root,
       required: requiredVariables,
@@ -120,6 +123,7 @@ class QueryAnalyzer {
 
     final query = _mapToDrift(_QueryHandlerContext(
       foundElements: foundElements,
+      elementReferences: references,
       queryName: declaration.name,
       requestedResultClass: requestedResultClass,
       requestedResultType: requestedResultType,
@@ -209,6 +213,7 @@ class QueryAnalyzer {
       context,
       root,
       queryContext.foundElements,
+      queryContext.elementReferences,
       updatedFinder.writtenTables
           .map((write) {
             final table = _lookupReference<DriftTable?>(write.table.name);
@@ -265,6 +270,7 @@ class QueryAnalyzer {
       context,
       queryContext.root,
       queryContext.foundElements,
+      queryContext.elementReferences,
       driftEntities,
       _inferResultSet(queryContext, resolvedColumns, syntacticColumns),
       queryContext.requestedResultClass,
@@ -447,6 +453,7 @@ class QueryAnalyzer {
     final driftResultSet = _inferResultSet(
       _QueryHandlerContext(
         foundElements: queryContext.foundElements,
+        elementReferences: queryContext.elementReferences,
         root: queryContext.root,
         queryName: queryContext.queryName,
         nestedScope: queryContext.nestedScope,
@@ -484,7 +491,7 @@ class QueryAnalyzer {
       _QueryHandlerContext queryContext, NestedQueryColumn column) {
     final childScope = queryContext.nestedScope?.nestedQueries[column];
 
-    final foundElements = _extractElements(
+    final (foundElements, references) = _extractElements(
       ctx: context,
       root: column.select,
       required: requiredVariables,
@@ -511,6 +518,7 @@ class QueryAnalyzer {
         requestedResultClass: resultClassName,
         root: column.select,
         foundElements: foundElements,
+        elementReferences: references,
         nestedScope: childScope,
       )),
     );
@@ -564,7 +572,7 @@ class QueryAnalyzer {
   ///    a Dart placeholder, its indexed is LOWER than that element. This means
   ///    that elements can be expanded into multiple variables without breaking
   ///    variables that appear after them.
-  List<FoundElement> _extractElements({
+  (List<FoundElement>, List<SyntacticElementReference>) _extractElements({
     required AnalysisContext ctx,
     required AstNode root,
     NestedQueriesContainer? nestedScope,
@@ -581,6 +589,8 @@ class QueryAnalyzer {
     final merged = _mergeVarsAndPlaceholders(variables, placeholders);
 
     final foundElements = <FoundElement>[];
+    final references = <SyntacticElementReference>[];
+
     // we don't allow variables with an explicit index after an array. For
     // instance: SELECT * FROM t WHERE id IN ? OR id = ?2. The reason this is
     // not allowed is that we expand the first arg into multiple vars at runtime
@@ -589,9 +599,15 @@ class QueryAnalyzer {
     var maxIndex = 999;
     var currentIndex = 0;
 
+    void addNewElement(FoundElement element) {
+      foundElements.add(element);
+      references.add(SyntacticElementReference(element));
+    }
+
     for (final used in merged) {
       if (used is Variable) {
         if (used.resolvedIndex == currentIndex) {
+          references.add(SyntacticElementReference(foundElements.last));
           continue; // already handled, we only report a single variable / index
         }
 
@@ -624,7 +640,7 @@ class QueryAnalyzer {
         final type = driver.typeMapping.sqlTypeToDrift(internalType.type);
 
         if (forCapture != null) {
-          foundElements.add(FoundVariable.nestedQuery(
+          addNewElement(FoundVariable.nestedQuery(
             index: currentIndex,
             name: name,
             sqlType: type,
@@ -657,7 +673,7 @@ class QueryAnalyzer {
           converter = (internalType.type!.hint as TypeConverterHint).converter;
         }
 
-        foundElements.add(FoundVariable(
+        addNewElement(FoundVariable(
           index: currentIndex,
           name: name,
           sqlType: type,
@@ -684,10 +700,10 @@ class QueryAnalyzer {
         // we don't what index this placeholder has, so we can't allow _any_
         // explicitly indexed variables coming after this
         maxIndex = 0;
-        foundElements.add(_extractPlaceholder(ctx, used));
+        addNewElement(_extractPlaceholder(ctx, used));
       }
     }
-    return foundElements;
+    return (foundElements, references);
   }
 
   FoundDartPlaceholder _extractPlaceholder(

drift_dev/lib/src/analysis/results/query.dart

@@ -103,6 +103,13 @@ enum QueryMode {
   atCreate,
 }
 
+///A reference to a [FoundElement] occuring in the SQL query.
+class SyntacticElementReference {
+  final FoundElement referencedElement;
+
+  SyntacticElementReference(this.referencedElement);
+}
+
 /// A fully-resolved and analyzed SQL query.
 abstract class SqlQuery {
   final String name;
@@ -143,22 +150,44 @@ abstract class SqlQuery {
   ///    if their index is lower than that of the array (e.g `a = ?2 AND b IN ?
   ///    AND c IN ?1`. In other words, we can expand an array without worrying
   ///    about the variables that appear after that array.
-  late List<FoundVariable> variables;
+  late final List<FoundVariable> variables =
+      elements.whereType<FoundVariable>().toList();
 
   /// The placeholders in this query which are bound and converted to sql at
   /// runtime. For instance, in `SELECT * FROM tbl WHERE $expr`, the `expr` is
   /// going to be a [FoundDartPlaceholder] with the type
   /// [ExpressionDartPlaceholderType] and [DriftSqlType.bool]. We will
   /// generate a method which has a `Expression<bool, BoolType> expr` parameter.
-  late List<FoundDartPlaceholder> placeholders;
+  late final List<FoundDartPlaceholder> placeholders =
+      elements.whereType<FoundDartPlaceholder>().toList();
 
   /// Union of [variables] and [placeholders], but in the order in which they
   /// appear inside the query.
   final List<FoundElement> elements;
 
-  SqlQuery(this.name, this.elements) {
-    variables = elements.whereType<FoundVariable>().toList();
-    placeholders = elements.whereType<FoundDartPlaceholder>().toList();
+  /// All references to any [FoundElement] in [elements], but in the order in
+  /// which they appear in the query.
+  ///
+  /// This is very similar to [elements] itself, except that elements referenced
+  /// multiple times are also in this list multiple times. For instance, the
+  /// query `SELECT * FROM foo WHERE ?1 ORDER BY $order LIMIT ?1` would have two
+  /// elements (the variable and the Dart template, in that order), but three
+  /// references (the variable, the template, and then the variable again).
+  final List<SyntacticElementReference> elementSources;
+
+  SqlQuery(this.name, this.elements, this.elementSources);
+
+  /// Whether any element in [elements] has more than one definite
+  /// [elementSources] pointing to it.
+  bool get referencesAnyElementMoreThanOnce {
+    final found = <FoundElement>{};
+    for (final source in elementSources) {
+      if (!found.add(source.referencedElement)) {
+        return true;
+      }
+    }
+
+    return false;
   }
 
   bool get _useResultClassName {
@@ -239,11 +268,12 @@ class SqlSelectQuery extends SqlQuery {
     this.fromContext,
     this.root,
     List<FoundElement> elements,
+    List<SyntacticElementReference> elementSources,
     this.readsFrom,
     this.resultSet,
     this.requestedResultClass,
     this.nestedContainer,
-  ) : super(name, elements);
+  ) : super(name, elements, elementSources);
 
   Set<DriftTable> get readsFromTables {
     return {
@@ -264,6 +294,7 @@ class SqlSelectQuery extends SqlQuery {
       fromContext,
       root,
       elements,
+      elementSources,
       readsFrom,
       resultSet,
       null,
@@ -372,10 +403,11 @@ class UpdatingQuery extends SqlQuery {
     this.fromContext,
     this.root,
     List<FoundElement> elements,
+    List<SyntacticElementReference> elementSources,
     this.updates, {
     this.isInsert = false,
     this.resultSet,
-  }) : super(name, elements);
+  }) : super(name, elements, elementSources);
 }
 
 /// A special kind of query running multiple inner queries in a transaction.
@@ -383,7 +415,11 @@ class InTransactionQuery extends SqlQuery {
   final List<SqlQuery> innerQueries;
 
   InTransactionQuery(this.innerQueries, String name)
-      : super(name, [for (final query in innerQueries) ...query.elements]);
+      : super(
+          name,
+          [for (final query in innerQueries) ...query.elements],
+          [for (final query in innerQueries) ...query.elementSources],
+        );
 
   @override
   InferredResultSet? get resultSet => null;
@@ -831,7 +867,7 @@ final class NestedResultQuery extends NestedResult {
 
 /// Something in the query that needs special attention when generating code,
 /// such as variables or Dart placeholders.
-abstract class FoundElement {
+sealed class FoundElement {
   String get dartParameterName;
 
   /// The name of this element as declared in the query
@@ -875,10 +911,8 @@ class FoundVariable extends FoundElement implements HasType {
   @override
   final bool nullable;
 
-  /// The first [Variable] in the sql statement that has this [index].
-  // todo: Do we really need to expose this? We only use [resolvedIndex], which
-  // should always be equal to [index].
-  final Variable variable;
+  @override
+  final AstNode syntacticOrigin;
 
   /// Whether this variable is an array, which will be expanded into multiple
   /// variables at runtime. We only accept queries where no explicitly numbered
@@ -900,38 +934,36 @@ class FoundVariable extends FoundElement implements HasType {
     required this.index,
     required this.name,
     required this.sqlType,
-    required this.variable,
+    required Variable variable,
     this.nullable = false,
     this.isArray = false,
     this.isRequired = false,
     this.typeConverter,
   })  : hidden = false,
-        forCaptured = null,
-        assert(variable.resolvedIndex == index);
+        syntacticOrigin = variable,
+        forCaptured = null;
 
   FoundVariable.nestedQuery({
     required this.index,
     required this.name,
     required this.sqlType,
-    required this.variable,
+    required Variable variable,
     required this.forCaptured,
   })  : typeConverter = null,
         nullable = false,
         isArray = false,
         isRequired = true,
-        hidden = true;
+        hidden = true,
+        syntacticOrigin = variable;
 
   @override
   String get dartParameterName {
     if (name != null) {
       return dartNameForSqlColumn(name!);
     } else {
-      return 'var${variable.resolvedIndex}';
+      return 'var$index';
     }
   }
-
-  @override
-  AstNode get syntacticOrigin => variable;
 }
 
 abstract class DartPlaceholderType {}

drift_dev/lib/src/writer/queries/query_writer.dart

@@ -755,9 +755,40 @@ class _ExpandedVariableWriter {
   _ExpandedVariableWriter(this.query, this._emitter);
 
   void writeVariables() {
-    _buffer.write('variables: [');
-    _writeNewVariables();
-    _buffer.write(']');
+    _buffer.write('variables: ');
+
+    // Some dialects don't support variables with an explicit index. In that
+    // case, we have to desugar them by duplicating variables, e.g. `:a AND :a`
+    // would be transformed to `? AND ?` with us binding the value to both
+    // variables.
+    if (_emitter.writer.options.supportedDialects
+            .any((e) => !e.supportsIndexedParameters) &&
+        query.referencesAnyElementMoreThanOnce) {
+      _buffer.write('executor.dialect.desugarDuplicateVariables([');
+      _writeNewVariables();
+      _buffer.write('],');
+
+      // Every time a variable is used in the generated SQL text, we have to
+      // track the variable's index in the second list
+      _buffer.write('[');
+      for (final source in query.elementSources) {
+        switch (source.referencedElement) {
+          case FoundVariable variable:
+            _buffer.write('${variable.index}, ');
+            break;
+          case FoundDartPlaceholder placeholder:
+            final context = placeholderContextName(placeholder);
+            _buffer.write('...$context.variableIndices');
+            break;
+        }
+      }
+
+      _buffer.write('])');
+    } else {
+      _buffer.write('[');
+      _writeNewVariables();
+      _buffer.write(']');
+    }
   }
 
   void _writeNewVariables() {

drift_dev/lib/src/writer/queries/sql_writer.dart

@@ -104,8 +104,8 @@ class SqlWriter extends NodeSqlBuilder {
   }
 
   FoundVariable? _findMoorVar(Variable target) {
-    return query!.variables.firstWhereOrNull(
-        (f) => f.variable.resolvedIndex == target.resolvedIndex);
+    return query!.variables
+        .firstWhereOrNull((f) => f.index == target.resolvedIndex);
   }
 
   void _writeMoorVariable(FoundVariable variable) {

drift_dev/test/writer/queries/sql_writer_test.dart

@@ -14,7 +14,7 @@ void main() {
   }) {
     final engine = SqlEngine();
     final context = engine.analyze(sql);
-    final query = SqlSelectQuery('name', context, context.root, [], [],
+    final query = SqlSelectQuery('name', context, context.root, [], [], [],
         InferredResultSet(null, []), null, null);
 
     final result = SqlWriter(options, dialect: dialect, query: query).write();

extras/integration_tests/drift_testcases/lib/database/database.g.dart

@@ -586,9 +586,12 @@ abstract class _$Database extends GeneratedDatabase {
           _ =>
             'SELECT COUNT(*) AS _c0 FROM friendships AS f WHERE f.really_good_friends = TRUE AND(f.first_user = ? OR f.second_user = ?)',
         },
-        variables: [
+        variables: executor.dialect.desugarDuplicateVariables([
           Variable<int>(user)
-        ],
+        ], [
+          1,
+          1,
+        ]),
         readsFrom: {
           friendships,
         }).map((QueryRow row) => row.read<int>('_c0'));
@@ -605,9 +608,13 @@ abstract class _$Database extends GeneratedDatabase {
           _ =>
             'SELECT f.really_good_friends,`user`.`id` AS `nested_0.id`, `user`.`name` AS `nested_0.name`, `user`.`birth_date` AS `nested_0.birth_date`, `user`.`profile_picture` AS `nested_0.profile_picture`, `user`.`preferences` AS `nested_0.preferences` FROM friendships AS f INNER JOIN users AS user ON user.id IN (f.first_user, f.second_user) AND user.id != ? WHERE(f.first_user = ? OR f.second_user = ?)',
         },
-        variables: [
+        variables: executor.dialect.desugarDuplicateVariables([
           Variable<int>(user)
-        ],
+        ], [
+          1,
+          1,
+          1,
+        ]),
         readsFrom: {
           friendships,
           users,