Queries run with `execute` don't use the global `typeCast` function
coreyjv opened this issue · 8 comments
Is this by design or a bug? Ideally I'd like to use prepared statements to absolutely prevent SQL injection attacks but I need to use a custom typecast function because of issues with timezone in #262.
Thanks
let's continue discussion here @aikar @coreyjv
suppose the data is coming as (number, date, string) field
What should happen if you call field.string() for first field for example? Read it as usual (as number) first, and then return cast as String(value)? What you expect as field.buffer() result for all three? I suspect underlying binary representation ( say, unsigned 32 bit int for first, 8 byte datetime, lengthcoded int prefixed string for second and third? )
If I place typeCast at upper level ( just before returning values, I think this is where it should be) it's much easier to implement but .buffer() and next() won't be available
Personally I only use the typecast because of #262 where it doesn’t seem like mysql2 respects the timezone configuration parameter. If I didn’t have any issues with DATETIME I’d not use typecast.
@sidorares are there any updates on this?
It's still not very obvious to me how to make it compatible between query() and execute()
In query results (almost) all fields are serialized as strings, so what typeCast api suggest is you call field.string() to get that internal string representation first and then use that however you want and cast to some other type
In execute result ( so called "binary protocol") each field type has it's own serialization format - length-prefixed 8/16/24/32/64 bit numbers, 8 byte datetime etc etc.
Execute might convert all this to string first when field.string() used but that would be mean double conversion and seems silly to me. On the other hand most use cases is to convert only small number of fields that are special to user for some reason so maybe not so silly.
wdyt @coreyjv ?
@sidorares like I mentioned in my comment if #262 can be fixed then I don't need this feature.
ok , I'll focus on fixing timezone options and we'll revisit this later
Closing due to #2398 🎉
Please feel free to ask anything.