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

Activity

1. warmwaffles commented on Apr 18, 2025

   

   warmwaffles

   on Apr 18, 2025

   Member

   More actions

   I'll look into this soon.
2. warmwaffles

   self-assigned this

   on Apr 18, 2025
3. warmwaffles commented on Apr 24, 2025

   

   warmwaffles

   on Apr 24, 2025

   Member

   More actions

   Indeed @LostKobrakai this looks to be a bug. Working on a fix now.
4. warmwaffles commented on Apr 24, 2025

   

   warmwaffles

   on Apr 24, 2025

   Last edited by warmwaffles

   Member

   More actions

   @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.
5. LostKobrakai commented on Apr 24, 2025

   

   LostKobrakai

   on Apr 24, 2025

   ContributorAuthor

   More actions

   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.
6. warmwaffles commented on Apr 24, 2025

   

   warmwaffles

   on Apr 24, 2025

   Member

   More actions

   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?
7. LostKobrakai commented on Apr 25, 2025

   

   LostKobrakai

   on Apr 25, 2025

   ContributorAuthor

   More actions

   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.
8. warmwaffles commented on Apr 25, 2025

   

   warmwaffles

   on Apr 25, 2025

   Member

   More actions

   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.
9. greg-rychlewski commented on May 10, 2025

   

   greg-rychlewski

   on May 10, 2025

   Contributor

   More actions

   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.