Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
LostKobrakai opened this issue Apr 15, 2025 · 7 comments
Open
Assignees

Comments

@LostKobrakai
Copy link
Contributor

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

@warmwaffles
Copy link
Member

I'll look into this soon.

@warmwaffles warmwaffles self-assigned this Apr 18, 2025
@warmwaffles
Copy link
Member

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

@warmwaffles
Copy link
Member

warmwaffles commented Apr 24, 2025

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

iex([email protected])10>     cond = dynamic([a], a.col in ^["abc", "def"])
dynamic([a], a.col in ^["abc", "def"])
iex([email protected])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([email protected])12> {sql, x} = Repo.to_sql(:all, query)
{"SELECT t0.\"col\" FROM \"table\" AS t0 WHERE (t0.\"col\" = ANY($1))",
 [["abc", "def"]]}
iex([email protected])16>     val = dynamic(^["abc", "def"])
dynamic([], ^["abc", "def"])
iex([email protected])17>     cond = dynamic([a], a.col in ^val)
dynamic([a], a.col in ^["abc", "def"])
iex([email protected])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([email protected])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.

@LostKobrakai
Copy link
Contributor Author

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.

@warmwaffles
Copy link
Member

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?

@LostKobrakai
Copy link
Contributor Author

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.

@warmwaffles
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants