denisenkom / pytds

Python DBAPI driver for MSSQL using pure Python TDS (Tabular Data Stream) protocol implementation
MIT License
192 stars 53 forks source link

Named Parameter substitution broken by wildcard #108

Closed allaboutmikey closed 4 years ago

allaboutmikey commented 4 years ago

When trying to use a parameterized query that also has a wildcard in it, the execute method fails when handling parameter substitution. The failure is on line 632 of __init_\.py which reads: operation = operation % rename I don't know what this step is trying to achieve, as there are no conversion specifiers in the operation string. Maybe if I were using format or pyformat parameter style, there would be one in there. If I skip that line in the debugger, the code continues and I get the result I am looking for. The following (simplified) example uses names from my database, but I suspect that the table and column names don't matter and you should be able to test this on any database because it doesn't get as far as executing the query anyway. cur.execute("select * from Orderstbl where OrderID=@order_id and OrderNumber like '%hf%'", {'order_id':1234})

denisenkom commented 4 years ago

Try:

cur.execute("select * from Orderstbl where OrderID=@order_id and OrderNumber like '%%hf%%'", {'order_id':1234})
denisenkom commented 4 years ago

Also take a look at this: https://www.python.org/dev/peps/pep-0249/#paramstyle

allaboutmikey commented 4 years ago

Thanks, I looked there originally when I was trying to figure out which paramstyle to use. For some reason I can't recall, I had trouble with the standard way and somewhere saw an example of someone using tsql's native format for parameters (WHERE name=@name). This has been working well for me till I got to this issue. Now that I re-read the code with pyformat in mind, I can see what the line is for. Obviously, by using tsql formatted operation strings, I was supplying the query in the format that the rename would have achieved. I'll have another go with pyformat, but I think your doubling up the % characters trick will be necessary either way.

allaboutmikey commented 4 years ago

Can confirm that %% works both with pre-formatted operation string with @name parameters and proper %(name)s pyformat parameters. Not sure why I was having trouble with pyformat style before, but must have been doing something else wrong back then. Any chance we can put in handling for this? operationstring = re.sub(r'%(?!\()','%%',operationstring) ie: replace all % with two if not followed by a ( This or something similar would mean you don't have to remember to work around the formatting issue when using wildcards (or other uses of % I suppose).

denisenkom commented 4 years ago

No, I don't think it would be a good idea to do such replacement. This would be non-standard and non-backward-compatible behavior.

allaboutmikey commented 4 years ago

I couldn't find any mention of the requirement to use double % in queries in the pep you linked to above. Where is this standard documented? I agree that the regex I provided would be non backward compatible, but it is easily modified to also leave existing %%s alone. Without looking at the code inside the db module, it's wasn't obvious to me that there was a % string substitution happening, so I didn't realise you'd have to use %% to get a % in the operation string. Is that explained anywhere in dbapi documentation or did everyone else know what was under the hood?

denisenkom commented 4 years ago

Pyformat is documented here: https://docs.python.org/2/library/stdtypes.html#string-formatting

allaboutmikey commented 4 years ago

Thanks for that link, I have looked there before. As in said previously, I didn't realise that dbapi just used string interpolation. I've had it in my head that the mechanics were different even thought the formatting is the same. Perhaps that was a limitation of a specific implementation. I'll close the issue because I'm just nit picking now. Thanks to your help, my original issue is solved. Great work on the driver by the way.