-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ToSql on Text hits SQL Server stack limit #32
Comments
Is the character escaper overly defensive? As a workaround I'm using a simpler scheme which only replaces |
It's overly defensive, yeah. Does your approach pass the property test suite? That might be a fine fix. The Right(tm) fix for this is to support binding parameters via the odbc API. That doesn't have to be an API change for the odbc package, either. As the The harder part is probably doing the necessary FFI call. I don't think it would be necessary for most of the types, but for text and binary it could be a significant security and performance boost. |
Tests for nvarchar pass with my approach but varchar has some problems. PR coming in a bit.
|
Does not pass tests.
I'm working on proper query parametrization (using the odbc API) this week. It'll be automatic and won't introduce an API change. |
I did write a little code for binding parameters after I reported this issue, feel free to use or not: spencerjanssen/odbc@e836897 I wasn't able to resolve round-trip issues with higher characters. The ODBC driver insists on treating C char strings as UTF-8 which means we can't just send a byte like |
@spencerjanssen Your code is very similar to mine, so that's good. We can't both be wrong. 😂 I'll keep an eye on the encoding issue, I'm using slightly different SQL types. I have to test some more, but got side-tracked on another task temporarily. |
The
ToSql
instance forText
can generate very large expressions of+
which throws the following error on my installation of SQL Server.The use-case that caused us to hit this was a modestly-sized HTML document, but here's a more minimal example:
The text was updated successfully, but these errors were encountered: