Repository navigation
Issues with precedence in generated SQL #181
Description
datetime_add(t, a + b, "day") generates the SQL a + b || ' day'. Because || binds tighter than +, this is not the desired SQL, and always evaluates to NULL. Instead, it should be (a + b) || ' day'. Likewise, perhaps worse, datetime_add(t, a + b, "week") adds (a + b * 7), not (a + b) * 7, days, and similarly for datetime_add(t, a + b, "millisecond").
(date_add also has these issues.)
While investigating the above, I found some additional, though lesser, precedence issues:
is_nil(x and y) becomes x AND y IS NULL (should be (x AND y) IS NULL), is_nil(not x) becomes NOT (x) IS NULL (should be (NOT x) IS NULL), (not x) < y becomes NOT (x) < y (should be (NOT x) < y), and likewise with (not x) in [...] and x == (y in [...]) (becomes x = y IN (...), and = binds tighter than IN).
Also, within update, inc: [x: a + b] has a minor precedence bug that only matters when addition is non-associative (e.g. floating point, or integer overflow.) It becomes UPDATE ... SET x = x + a + b, which means x = (x + a) + b, but it should instead be x = x + (a + b).
Activity
warmwaffles commentedon Sep 20, 2026
I wonder if this effects postgres and mysql as well. They have similar functionalities. 🤔
warmwaffles commentedon Sep 20, 2026
@foxbenjaminfox can you take a look at #182? Maybe if you can come up with another obscure test case as well I can chuck that into the test suite as well.
Parenthesize SQL expressions with lower precedence
foxbenjaminfox commentedon Sep 20, 2026
I went looking for similar issues in the postgres and myxql adapters, and filed an issue with what I found there: elixir-ecto/ecto_sql#754. That may be a reasonable source of additional test cases. e.g.
(x == y) in [1, 2], a boolean operator on the left of anin.I see that #182 also parenthesized the subquery
incase: that could get a test covering it too, e.g.(x + y) in subquery(...).Fragments as operands are another tricky issue, one which in the general case I do not expect the adapter to deal with. e.g.
is_nil(fragment("? OR ?", x, y))still becomesx OR y IS NULL, but there you could quite reasonably argue that it is the user's responsibility to add the parentheses in the fragment. Butdatetime_addand similar are a tougher question, because the user cannot see the implementation details—datetime_add(s.x, fragment("? + ?", y, z), "day")results inx + y || ' day', and if the user doesn't know that this is the implementation they silently get a bad result for no real reason they can see. I don't know how you want to handle that.