findmypast-oss / mssql_ecto

Ecto adapter for Mssqlex
Apache License 2.0
49 stars 20 forks source link

Possible issue with query() #12

Closed mfeckie closed 7 years ago

mfeckie commented 7 years ago

When trying to execute a parameterised query using Ecto.Adapters.SQL.query, I receive an error message that the number of parameters is incorrect (1 for 0), irrespective of how many are present in the string.

This block seems to be the cause

https://github.com/findmypast-oss/mssql_ecto/blob/master/lib/mssql_ecto/connection.ex#L73-L93

I'm not entirely sure I understand what the Regex is attempting to do, particularly as both capture groups capture the same section.

The second Regex which I assume is supposed to count the number of $x parameters doesn't seem to be working either.

I suspect I'm missing something, could someone help my understand where I'm going wrong?

Thanks

jbachhardie commented 7 years ago

How are you writing the query? The likely issue is that you're writing INSERT INTO foo (var1, var2) VALUES (?, ?) which is the ODBC standard but MssqlEcto, for reasons of compatibility with Ecto's DSL, expects parameters to have an ordering, i.e. INSERT INTO foo (var1, var2) VALUES (?1, ?2).

Does doing it the second way solve your issue?

I'd actually like for Ecto.Adapters.SQL.query to accept the standard form but that would require some code changes I think. We've never used parametrised queries in this way.

mfeckie commented 7 years ago

Yep, that works. Thanks.