[Python] Add support for named parameters #5582
Conversation
| param_name); | ||
| } | ||
| // Replace all occurences of $name with $<index> | ||
| query = StringUtil::Replace(query, param_name, StringUtil::Format("$%d", param_idx)); |
There was a problem hiding this comment.
Very much an edge case, but what if $variable appears inside a string in the query?
There was a problem hiding this comment.
Ah I thought about this but only in relation to column names, which I reasoned cant contain $, but you raise a valid point, string literals can contain this..
Definitely needs a test for this and probably requires changing the implementation 👍
There was a problem hiding this comment.
My main concern is for people using regex's, where '$hello' is perfectly valid regex
There was a problem hiding this comment.
Yes, definitely, already said you're right 👍
Need to change the implementation
There was a problem hiding this comment.
Yep just giving a realish query example that could break
|
Hmm it seems this does come at the cost of existing functionality, since import duckdb
con = duckdb.connect()
con.execute("create table tbl (i integer)")
con.execute("insert into tbl VALUES (?), (?), (?)", {2:5,3:6,5:4})
res = con.execute("select * from tbl").fetchall()
print(res)
# [(2,), (3,), (5,)]currently works on master |
It seems unlikely that people are using it like this, and the fix is pretty straightforward - |
|
I would agree that few folks are likely to be using a dict's keys to pass in params! |
This PR implements named parameters (mentioned in #5388)
Basically what happens is we now detect if the passed object to
paramsis adict, and if it is we assume these contain named parameters, so we replace$param_namewith$1in the query string and populate the param list with the corresponding values of the dict.Test todos:
ExecuteMany tests
queries containing the named parameter in strings
Also need to check if there is no existing functionality for dictionary as
paramthat gets broken by this change