elixir-sqlite/ecto_sqlite3

Issue with parameters in relation to in and a dynamic/1 collection

Opened this issue · 8 comments

Doesn't work:

val = dynamic(^["abc", "def"])
cond = dynamic([a], a.col in ^val)
query = from a in "table", where: ^cond, select: a.col

{sql, x} = Repo.to_sql(:all, query)
# sql = SELECT t0."col" FROM "table" AS t0 WHERE (t0."col" IN (?,?))
# x = [["abc", "def"]]

Works:

cond = dynamic([a], a.col in ^["abc", "def"])
query = from a in "table", where: ^cond, select: a.col

{sql, x} = Repo.to_sql(:all, query)
# sql = SELECT t0."col" FROM "table" AS t0 WHERE (t0."col" IN (?,?))
# x = ["abc", "def"]

Looks like there's some place flattening out the parameters, which doesn't catch the dynamic case

I'll look into this soon.

Indeed @LostKobrakai this looks to be a bug. Working on a fix now.

@LostKobrakai the "works" is different behavior than the postgres adapter because the ANY keyword doesn't appear.

iex(app@127.0.0.1)10>     cond = dynamic([a], a.col in ^["abc", "def"])
dynamic([a], a.col in ^["abc", "def"])
iex(app@127.0.0.1)11>     query = from a in "table", where: ^cond, select: a.col
#Ecto.Query<from t0 in "table", where: t0.col in ^["abc", "def"],
 select: t0.col>
iex(app@127.0.0.1)12> {sql, x} = Repo.to_sql(:all, query)
{"SELECT t0.\"col\" FROM \"table\" AS t0 WHERE (t0.\"col\" = ANY($1))",
 [["abc", "def"]]}
iex(app@127.0.0.1)16>     val = dynamic(^["abc", "def"])
dynamic([], ^["abc", "def"])
iex(app@127.0.0.1)17>     cond = dynamic([a], a.col in ^val)
dynamic([a], a.col in ^["abc", "def"])
iex(app@127.0.0.1)18>     query = from a in "table", where: ^cond, select: a.col
#Ecto.Query<from t0 in "table", where: t0.col in ^["abc", "def"],
 select: t0.col>
iex(app@127.0.0.1)19> {sql, x} = Repo.to_sql(:all, query)
{"SELECT t0.\"col\" FROM \"table\" AS t0 WHERE (t0.\"col\" = ANY($1))",
 [["abc", "def"]]}

I'll need to figure something out here.

Postgres uses the array as parameter because it's also doing ANY($1). The sqlite query uses multiple parameters (?, ?), so flattening the list into the top level parameters makes sense.

Yea we can't use the [["ABC", "DEF"]] construct because sqlite has no ability to bind a list of values. https://sqlite.org/c3ref/bind_blob.html

This may require upstream changes in Ecto. I'm still trying to figure out a solution. As a work around, don't do this:

val = dynamic(^["abc", "def"])
cond = dynamic([a], a.col in ^val)

I am curious, why is that being used rather than just passing the list directly?

I am curious, why is that being used rather than just passing the list directly?

This is for a query builder, where conditions come from a datastructure. The right hand side of in could also be another expr instead of a value, hence the separation.

I do not believe this can be supported. The list of array values without changing things upstream in ecto. All of the other SQL implementations can handle it just fine because ANY(?) is supported, but we do not have access to it in sqlite and we also do not have the ability to flatten that param list before being passed to the query preparation state. Unsure on how to raise a more helpful exception stating that this can not be supported. I don't want to break a dynamic built condition from being used.

So the work around for now is to not use dynamic in this way.

For some more background why this happens. The nested dynamic does not "see" the outer dynamic. So it doesn't know it's part of an in expression. The in expression is handled specially by the Ecto planner to split it up, which is why not nesting the dynamics works.

So this could theoretically be supported of we can make inner dynamics see the context they are in. But I imagine this is not a simple change.