Repository navigation
Is it possible to pass query parameters? (:param or ?) #513
Description
Activity
Yea, parameterized queries would be awesome, especially syntax like this:
ctx.sql( "select country, count(*) as num_ppl from {person_df} group by country", person_df=person_df, ).show()
I don't fully understand what is needed here. Can you not just do
ctx.sql( f"select country, count(*) as num_ppl from {person_df} group by country" ).show()@timsaucer - In this example,
person_dfis a DataFrame object, not a table name. This saves the user from having to register the table (the engine just does this under the hood). It was recently added to Spark and is a surprisingly nice dev quality of life improvement!Oh, interesting. As a pure dataframe user, I hadn't understood the request. I appreciate the clarification!
Parametrized queries are even more important for when you want to do:
WHERE col IN big_list_of_values.A year later, I again need to choose a library for reading parquet files, but I had to settle with ducks because data fusion doesn't support this
I just tried a quick proof of concept. To get this to work, I did need a name for the table. Would the following be an acceptable solution?
from datafusion import SessionContext ctx = SessionContext() ctx.register_parquet("customers", "examples/tpch/data/customer.parquet") df_customer = ctx.table("customers") ctx.sql("select c_custkey, c_name from {df}", df=df_customer)I think that if we change all of the
read_xlikeread_parquetover to callregister_xand auto generate a table name, then we can support removing the register line and make this work. I tested with parquet with success.@kylebarron - I prefer the Spark parameterized syntax because it's not reliant on local scope. I've fought with the DuckDB syntax in the past cause you can lose the scope when you move to a new function, for example.
BTW, it'd be cool if we could parameterize lots of different values, not just DataFrames. This would have been a better example:
from datetime import date ctx.sql(""" SELECT country, COUNT(*) as num_ppl FROM {person_df} WHERE date_column > {cutoff_date} GROUP BY country """, person_df=person_df, cutoff_date=date(2020, 1, 1) ).show()
The extra bonus point implementation would be if the user could add custom parameterization logic for their own objects!
I think parameterization is important, but I think most important for strings to avoid SQL injection.
I think Arrow parametrization could make sense, but I think it should be a second PR after.
WHERE date_column > {cutoff_date}It's not clear to me that there's a 1:1 correspondence between a Python date and the correct SQL string. I think having arbitrary parameterization logic would be hard. (Or you could tell the user to implement
__str__, but then we're back to string interpolation.@kylebarron - yea, here's a blog post on how PySpark does the parameterization. Not all Python objects are parameterizable, just the common ones used in SQL queries. You're right that the parameters need to get sanitized - thanks for calling that out!!
Should there be another issue dedicated to query sanitization? I'm not an expert on what needs to be done to achieve that
We have two issues and two PRs addressing this vs direct replacement.
#1244 requests replacement scans and #1247 addresses it.
#964 addresses this issue and was brought up again today at the community meeting.
@MrPowers and @kylebarron as the two people requesting these features, can you give a better sense of how strongly you feel about us going one direction or the other? I don't think we should try to do both.
Tagging @kosiew who has done a lot of work on his PR.
Is there a tl;dr summary of the difference between the two PRs?
From a user experience perspective, we have this issue which has parameterized queries like:
ctx.sql("select c_custkey, c_name from {df}", df=df_customer)And the similar query for direct injection would be
ctx.sql("select c_custkey, c_name from df_customer")As Matt points out above we could potentially do replacement of things other than table names with the parameterization.
From an implementation perspective, the parameterization approach searches any SQL command for
{}and matching named arguments and does a replacement with their string equivalent of their logical plan.The direct injection tries to execute the SQL command, and on failures then tries to find objects that can be table providers.
Reacted by Kyle BarronI like this syntax
ctx.sql("select c_custkey, c_name from {df}", df=df_customer).The
ctx.sql("select c_custkey, c_name from df_customer"), which relies on adf_customervariable being defined in the local scope works ok in a notebook, but breaks down in an application with functions where the local scope is changing a lot.I ran into the pain point of direct injection when using DuckDB in a project. I was reading the tables in one function and then trying to access them in another function, but they were not accessible. Let me double check this tho.
Spark & Daft use parameterized queries, which work better, in my experience.
Reacted by Adrian Garcia Badaracco and Kyle BarronI think it's fine to be explicit about query parameters instead of using implicit replacement scans.
My only question is if there's a syntax that is in wide existing use, so that we don't create a slightly derived SQL dialect that is incompatible with what people are used to. I've seen
:paramand?before; should we use one of those instead of creating{variable}?I've been using Claude to assist me in trying to understand the conventions, so take this with a grain of salt.
- DuckDB:
result = duckdb.sql("SELECT * FROM df", df=pandas_df)(but we've seen they can also pull directly from scope) - Spark:
spark.sql("SELECT * FROM users WHERE age > :age", age=25) - Daft:
daft.sql("SELECT * FROM df", catalog={"df": df_customer}) - Pandas:
query = "SELECT * FROM df_customer"(direct injection it looks like) - Postgresql:
cursor.execute("SELECT * FROM users WHERE age > %s", (25,))or with named parameterscursor.execute("SELECT * FROM users WHERE age > %(age)s", {"age": 25})
One potential problem with the proposed
ctx.sql("select c_custkey, c_name from {df}", df=df_customer)is that if the user also uses f-string replacement it gets messy. For example suppose they didctx.sql(f"select {key_of_interest}, c_name from {df}", df=df_customer)then I expect this would go very poorly. It would try to coercedfto a string because of thef" ".I'm a bit torn on the PostgreSQL approach. On the one hand
datafusionupstream tries to stick closely to PostgreSQL. On the other hand the non-named parameters I find to be nasty. It reminds me of oldfprintstatements where you had to closely watch your parameter ordering.From this preliminary look it doesn't appear that there is a strong consensus in approach.
If I had to pick from one of these, I would probably lean towards the Spark approach. I think the f-string replacement argument is a very valid one and would just lead to headaches down the road for our users.
Reacted by Kyle BarronReacted by Kyle Barron- DuckDB:
For example suppose they did
ctx.sql(f"select {key_of_interest}, c_name from {df}", df=df_customer)then I expect this would go very poorlyYes, you'd need to know to escape the f string with a double
{{}}ctx.sql(f"select {key_of_interest}, c_name from {{df}}", df=df_customer)
Reacted by Tim Saucer and kosiewI've gone down a deep rabbit hole, but I think I have a path forward. Instead of doing string replacement, we should probably use the sql parser to ensure we aren't doing any kind of malicious injection of code. One way you could do something would be to create a table and register it with table name
t; drop table t;and if you did string replacement in the SQL you could lead to bad behavior.Instead I plan to use the sql parser crate to pre-process the sql string into tokens. We can't just use the placeholder evaluation of the parser because it does not expect table names to be valid placeholders.
I think the path forward is (mostly notes for myself):
- Convert sql string to tokens.
- Search for Tokens that are
Placeholder("$df")orWord(Word { value: "$df", quote_style: None, keyword: NoKeyword })to cover all the dialects in the sql parser. - Validate that the temporary table name we create parses back into a single
Wordtoken. - If those conditions match replace the matched token.
- Build SQL from this.
- For all non-dataframe elements we can then use the sqlparser to then do the appropriate scalar value casting as appropriate.
Instead of doing string replacement, we should probably use the sql parser to ensure we aren't doing any kind of malicious injection of code
💯
Hey, I'm trying to port a query from
duckdbtodatafusionusing the Python connector, the issue I'm facing, is adding query parameters.Is there currently a way that I can pass query parameters ?