Possible incorrect parentheses handling in SQL expressions

[Andre-lbc]

Problem:

I'm encountering an issue with Drift where, under certain circumstances, the generated SQL does not include the necessary parentheses in expressions that should be arithmetically equivalent. This leads to incorrect query results due to operator precedence errors.

Context:

I have a Tab table with a position column that has a unique constraint. To update positions without violating this constraint (e.g., when moving tabs, deleting multiple tabs, etc), I temporarily assign negative positions to affected rows. After moving the necessary items, I normalize any negative positions back to their final positive positions.

Code samples:

await db.transaction(() async {

  // ...

  final int moveBy = 10;

  // Case 1: Works as expected
  // Generated SQL:
  // UPDATE "tb_tab" SET "position" = ? * ("position" - ?) WHERE "position" < ?; with args [-1, 10, 0]
  await (db.update(db.tbTab)
        ..where((tab) => tab.position.isSmallerThanValue(0)))
      .write(
    TbTabCompanion.custom(
      position: const Variable(-1) * (db.tbTab.position - Variable(moveBy)),
    ),
  );

  // Case 2: Works as expected
  // Generated SQL:
  // UPDATE "tb_tab" SET "position" = 0 - "position" - 10 WHERE "position" < ?; with args [0]
  await (db.update(db.tbTab)
        ..where((tab) => tab.position.isSmallerThanValue(0)))
      .write(
    TbTabCompanion.custom(
      position: -(db.tbTab.position - Variable(moveBy)),
    ),
  );

  // Case 3: Does not work as expected
  // Generated SQL:
  // UPDATE "tb_tab" SET "position" = -("position" - 10) WHERE "position" < ?; with args [0]
  // Problem: Missing parentheses affect the results. The expression is evaluated differently than intended.
  // Example: 0 - (15 - 10) = -5, but 0 - 15 - 10 = -25
  await (db.update(db.tbTab)
        ..where((tab) => tab.position.isSmallerThanValue(0)))
      .write(
    TbTabCompanion.custom(
      position: const Variable(0) - (db.tbTab.position - Variable(moveBy)),
    ),
  );
});

Note: Using Constant() instead of Variable() or in addition to it generates the same problems.

[simolus3]

Good catch! This has probably been broken since forever. Drift adds parentheses if inner expressions have a higher precedence than outer ones:

  var x = a + b
  return x * 3 // Generates (a + b) * 3

Doing this only when the inner precedence is higher is not correct when non-associative operators are involved. For safety, we should add parentheses if the inner precedence matches the outer one too.