-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Wiki and FAQ disagree over prepared statement behavior #345
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
Comments
Just to comment, the most useful thing from a user perspective would be to only prepare if either:
Example code for naively achieving this by inheriting from Client: function PreparedStmtClient(config)
{
this.__preparedStatements = {};
pg.Client.apply(this, arguments);
};
util.inherits(PreparedStmtClient, pg.Client);
PreparedStmtClient.prototype.query = function query(config, values, callback)
{
if(config && config.name && config.text)
{
if(this.__preparedStatements[config.name] == config.text)
{
// We've already prepared this statement; delete config.text so we don't re-prepare.
delete config.text;
}
else
{
// We'll be preparing this statement; add it to the cache so we can tell that it's been prepared.
this.__preparedStatements[config.name] = config.text;
}
}
return pg.Client.prototype.query.call(this, config, values, callback);
}; Of course, in order to be production-ready, this would also need error handling so we don't mark a statement as prepared if it errors. If there's no objections, I might put together a pull request implementing this in |
👍 as long as it's tested it'll be happily accepted |
Just to clarify, which place accurately represents the current behavior? Either the docs or the FAQ should probably be corrected in the meantime. |
updated documentation |
Can you clarify this on the wiki too? At a glance, it looks like no code was committed for this, and the wiki is alarming. https://github.com/brianc/node-postgres/wiki/Prepared-Statements |
How is it alarming? What do you want me to clarify? |
I had the same confusion, but it looks like it works as you would hope. If you execute a query as { name: 'my-latest-posts', text: 'SELECT * FROM posts WHERE user_id = $1 ORDER BY updated_at DESC LIMIT 10', values: [1] } node-postgres keeps a track, per connection (which is how Postgres handles them), that 'my-latest-posts' has been prepared (and prepares the statement on that connection if not). Line 134 of query.js : connection.parsedStatements[this.name] = true; |
@brianc Just as an added note: this works on a per-connection basis, so as I am using connection pooling, I find it is needing to create a couple of PS to start with, then keeps reusing them. However if I back off for a while and then start using them again, the connections have died so it needs to re-create the PS. So, I never really know when I should be passing text to it, so I just always am. I can see in the code that it checks to see if it needs to create something for the given name on the connection. What confused me and led me to dig into this was the following on the wiki here, https://github.com/brianc/node-postgres/wiki/Client#wiki-method-query-prepared
I thought that mean that to execute a prepared statement a second time, but I needed to explicitly not pass the text, since I didn't realise that the statements were being cached on a connection level, not a library level. The other thing that is confusing is the connection pooling issue. If you could perhaps give another example, in addition to the excellent single connection example in the above link, about how to handle it with connection pools (ie always pass the text) this would be nice! |
Just as a further note, I identified whether my connections were being cached or not by changing the prepare function as so:
|
@brianc now I'm looking at it, trying to figure out how to do the following... I have a Prepared Statement object that wraps it and returns an object automatically. I want to make it set Question: Is there such thing within the current connection that can be used to uniquely identify the connection? |
@brianc I think I got it figured out - |
You might want to update wiki/Prepared-Statements referring to this issue |
From the FAQ:
From the wiki:
So, the question is... which one accurately represents node-postgres's behavior? The wiki and/or FAQ should be updated to reflect the actual behavior.
The text was updated successfully, but these errors were encountered: