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

Parameterization Enhancement #117

Closed
steelbrain opened this issue Aug 30, 2014 · 11 comments
Closed

Parameterization Enhancement #117

steelbrain opened this issue Aug 30, 2014 · 11 comments

Comments

@steelbrain
Copy link

The module is great but it would be great to have the PHP-PDO style parameterization. At the moment an examaple sql query is Select 1+? as sum, this becomes confusing with more multiple parameters Select users.json, EXISTS(Select 1 from moderators where moderators.id = ?) as is_moderator from users where users.id = ? and users.status = ? and users.complete_status = ?.

However in pdo, it's always simple and you can use the same parameter at multiple locations Select users.json,EXISTS(Select 1 from moderators where moderators.id = :id) as is_moderator from users where users.id = :id and users.status = :status and users.complete_status = :complete_status.

@sidorares
Copy link
Owner

Unfortunately named placeholders are not supported by mysql and this has to be done client side. This is doable (and I probably would merge if someone can make it) but main focus for this module is to be as fast and low level

In you example, driver would have to convert Select users.json,EXISTS(Select 1 from moderators where moderators.id = :id) as is_moderator from users where users.id = :id and users.status = :status and users.complete_status = :complete_status + {id: id, status: status, comple_status: complete_status} into Select users.json, EXISTS(Select 1 from moderators where moderators.id = ?) as is_moderator from users where users.id = ? and users.status = ? and users.complete_status = ? + [ id, id, status, complete_status, status] (yes, serialising id and status multiple times)

@steelbrain
Copy link
Author

It would be a great feature though, does it eat too much performance?

@sidorares
Copy link
Owner

No, not at all, this would be one-time client side compilation step (cached for subsequent calls). In fact, https://github.com/mscdex/node-mariasql is doing this and it might be possible to reuse named placeholders parsing code from there. Ping @mscdex :)

@sidorares
Copy link
Owner

The code is here - https://github.com/mscdex/node-mariasql/blob/master/lib/Client.js#L272-L332
I might have some time today to try to port it

@sidorares
Copy link
Owner

I extracted code - this should be enough: https://gist.github.com/sidorares/bb1f3ba0274a6ce3f961

Now I need to integrate into library (making sure that if not used there is no performance penalty)
Probably behind 'useNamedPlaceholders' config flag. Also need to cache results. Feel free to try to do this yourself. I might put 'named to unnamed placeholders' into separate module so others can reuse it. Not sure if caching layer should be part of that module

@steelbrain
Copy link
Author

awesome, gonna give it a try

@sidorares
Copy link
Owner

Post your examples here, I'll add them as tests to module

@sidorares
Copy link
Owner

@steelbrain
Copy link
Author

I tried to understand how the node modules work, but couldn't (possibly because of lack of time). and that plugin is awesome. Thanks

@davehorton
Copy link

This seems to break this feature from the underlying mysql library

var post  = {id: 1, title: 'Hello MySQL'};
var query = connection.query('INSERT INTO posts SET ?', post, function (error, results, fields) {
  if (error) throw error;
  // Neat!
});

I now get an error along the lines of "bind parameters must be an array"

@sidorares
Copy link
Owner

@davehorton I don't think this is related to named placeholders feature

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

3 participants