livebook-dev / kino_db

Database integrations for Livebook
Apache License 2.0
40 stars 16 forks source link

SQL Variable Injection WHERE IN Clauses #76

Closed rcuevas-wash closed 4 weeks ago

rcuevas-wash commented 2 months ago

Environment

Elixir 1.16.2 (compiled with Erlang/OTP 26)

Current behavior

SQL Intgration, MSSQL When you want to filter On multiple values for a column such as:

SELECT * FROM users WHERE id IN ( {{ user_ids }} )

It is not clear how to pass a collection into IN (...) Arrays, tuples, char lists, strings.. there is no data type or collection of data types that allows one to do this type of query successfully. It returns the following error:

** (FunctionClauseError) no function clause matching in Tds.Parameter.fix_data_type/1

Expected behavior

user_ids can be any of the following values:

[ '5', '6' ], "'5', '6'", "\"5\", \"6\"", { '5','6' }.. etc etc

And so the following query would work:

SELECT * FROM users WHERE id IN ( {{ user_ids }} )

josevalim commented 2 months ago

Unfortunately we cannot pass lists as parameters, I am not sure there is a specific solution we could employ here. One possible approach would be for you to encode the IDs as a string and then string split the string:

SELECT *
FROM users
WHERE id IN (SELECT value FROM STRING_SPLIT({{ Enum.join(user_ids) }}, ','));

@jonatanklosko we could try to generate the SQL dynamically but that would require us to read the value (the list size) at the time we generate code, and I am not quite sure if it is possible?

rcuevas-wash commented 2 months ago

Thank you for your reply. I tried the above, STRING_SPLIT({{ Enum.join(user_ids) }}, ','), and it did not work. There is no explicit error it just returns and empty set with every possible permutation of "encode the IDs as a string"

What you proposed above does work in phoenix , using raw sql and ecto with tds(mssql), but not here.

jonatanklosko commented 2 months ago

we could try to generate the SQL dynamically but that would require us to read the value (the list size) at the time we generate code, and I am not quite sure if it is possible?

@josevalim we could scan the binding to know which variables are lists, so we could special case {{list_var}}. That's not reliable though, because someone can do {{Enum.map(...)}} and we wouldn't know it's a list. Handling one case without the other doesn't seem correct.

jonatanklosko commented 2 months ago

@rcuevas-wash oh, I think it's supposed to be Enum.join(user_ids, ","), please try that :D

rcuevas-wash commented 2 months ago

No Cigar. Im going to in the meantime do the filtering in elixir for multiple values.

jonatanklosko commented 2 months ago

@rcuevas-wash to make sure we are on the same page, can you try this query?

SELECT * FROM sys.Databases
WHERE database_id IN (SELECT value FROM STRING_SPLIT({{ Enum.join([1, 3], ",") }}, ','))

Seems to be working as expected for me:

image

FTR, I am using this image:

docker run -e "ACCEPT_EULA=Y" -e "MSSQL_SA_PASSWORD=reallyStrongPwd123" \
  -p 1433:1433 --name sql2022 --hostname sql2022 \
  --rm \
  mcr.microsoft.com/mssql/server:2022-latest
rcuevas-wash commented 4 weeks ago

This works. Can close Thank you !