Skip to content
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

typeCast not supported with .execute #649

Closed
aikar opened this issue Oct 10, 2017 · 8 comments
Closed

typeCast not supported with .execute #649

aikar opened this issue Oct 10, 2017 · 8 comments

Comments

@aikar
Copy link
Contributor

aikar commented Oct 10, 2017

using typeCast option in .query works, but does not work in .execute. The field is not type casted.

@sidorares
Copy link
Owner

typeCast only documented to work with .query()

The aim of typeCast() is to 1) give access to internal raw representation of data and 2) conrol what's generated out of this raw data

With .execute() this raw data is completely different ( prepared statements use so called 'binary protocol' and plain queries 'text protocol' )

Also I think current api of typeCast is flawed: you as consumer responsible of reading all raw data in full and in correct order. If you miss something everything breaks.

@aikar
Copy link
Contributor Author

aikar commented Oct 17, 2017

@sidorares I understand why it's more difficult to do, but the options really should work for both methods...

I ultimately had to re-implement typeCast in an abstraction layer that any time .execute is called on my layer, it immediately iterates the fields response and does type conversion.

For example, someone decided to use a VARBINARY column type for string keys in order to enforce case sensitivity (very old table before mysql supported this natively), and I now need to convert that to string.

A solution is to simply take the fields metadata response, and rebuild the same data structure the .query based call is supplied.

@sidorares
Copy link
Owner

I'll try to check how easy to do that, maybe actually not that difficult

@sidorares sidorares reopened this Oct 17, 2017
@ssypi ssypi mentioned this issue Nov 28, 2017
BinaryFissionGames added a commit to skuhl/RobotRemote that referenced this issue Jun 4, 2018
This is necessary due to
sidorares/node-mysql2#649
We could just cast wherever we call execute, instead of using the
typecast option, but this is easier,
and we shouldn't really NEED prepared statements
(though they would be nice)
@javiertury
Copy link

I am trying to make Sequelize use binded parameters here. The Sequelize team prefers node-mysql2 as a mysql driver, but unfortunately Sequelize needs typeCast() functionality.

Also, the overall information returned by the execute() method is poorer than for query().

@javiertury
Copy link

Apart from applying timezone, typeCast is also useful to convert geometries to geoJSON format. If it is not possible to add typeCast support for .execute, perhaps some of these common typeCasts can be implemented as built-in options for node-mysql2, e.g. options.geoJSON = true.

@trevorr
Copy link
Contributor

trevorr commented Aug 9, 2019

I've been using typeCast to convert bit(1) columns from Buffer to boolean. I couldn't figure out why it stopped working until I realized a changed query to execute and found this issue. Even if it there's not an easy fix, it would be a big help to document the difference and possibly generate a warning the first time execute is used on a connection with a typeCast. I'll submit a PR for either or both if you want @sidorares.

While I'm on the subject, would it be reasonable to have a built-in conversion for bit(1) returned as boolean? For compatibility, it could be enabled via an option. I could submit a PR for that as well.

@wellwelwel

This comment was marked as resolved.

@wellwelwel
Copy link
Collaborator

Closing due to #2398 🎉

Please feel free to ask anything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants