Skip to content
This repository has been archived by the owner on Mar 4, 2019. It is now read-only.

Suggestion: Add node-postgres-named to easily support named parameters #206

Closed
n321203 opened this issue Feb 5, 2016 · 8 comments
Closed
Milestone

Comments

@n321203
Copy link

n321203 commented Feb 5, 2016

I would suggest adding support for node-postgres-named
It patches the pg-client to support named parameters (what it does is looking at the named parameters and write a regular array of parameters [$1, $2, $3 etc]...).

That way, we can write more readable queries:

(insertUser.sql):
INSERT INTO users VALUES ($firstname, $lastname)

(now we can write):
db.insertUser( {firstname: 'Peter', lastname: 'Pan'},  fn(...) )

Much more readable than writing queries with $1, $2 etc!

@xivSolutions
Copy link
Collaborator

Interesting! Definitely worth looking into...so long as it doesn't mess up any of the internal plumbing in Massive.

On the other hand, taking on another external dependency is something to consider carefully as well.

@n321203
Copy link
Author

n321203 commented Feb 5, 2016

Yup, I agree. It's actually only ~70 lines of code so another option is to implement something similar directly into massive (and give credit to the author...).

(Update: link is now working)

@xivSolutions
Copy link
Collaborator

Ha ha - Yep. Stealing, with some attribution ("code re-use" ;-)), was my thought. That way we can make it available through the API where it makes sense, but retain the indexed params if/where that makes sense.

@dmfay
Copy link
Owner

dmfay commented Feb 5, 2016

This would definitely be cool for scripts & functions (table crud should stick with indexed parameters imo), but it's going to be difficult to implement or integrate there because of how Executable.invoke() sorts out its arguments.

You can pass up to three args: parameters (primitive or array), options (object; currently just used to toggle results streaming), and callback. Since both parameters and options are optional, it checks the types of what you pass it and assumes an object is the options and anything else is the parameters. When using this, the parameters could also be an object, and I don't know if it's possible to resolve that ambiguity while remaining backwards-compatible.

@n321203
Copy link
Author

n321203 commented Feb 6, 2016

Hmm... sounds like it is impossible without breaking backwards compatibility (if we don't look at the objects and assume the first found object containing the key "stream" is the options object... but that's ugly!)

@n321203
Copy link
Author

n321203 commented Feb 6, 2016

I would probably change the arguments/signature to:

  • If one argument is passed, assume it is the parameters,
  • If two arguments is passed, require the first to be the parameters, and the second one to be the options.

That way, the api will change only for people using streams today without any parameters (passing the options object as first argument)

@emfax
Copy link

emfax commented May 26, 2016

I also think the named parameters thing would be pretty cool. I had a reason to use overloaded SQL functions and named parameters would help that. At the moment, there doesn't seem to be support for overloaded SQL functions.

@dmfay
Copy link
Owner

dmfay commented May 23, 2017

It's happening! At least for run and script files!

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

No branches or pull requests

5 participants