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

Wiki and FAQ disagree over prepared statement behavior #345

Closed
whitelynx opened this issue May 2, 2013 · 12 comments
Closed

Wiki and FAQ disagree over prepared statement behavior #345

whitelynx opened this issue May 2, 2013 · 12 comments

Comments

@whitelynx
Copy link
Contributor

From the FAQ:

If a prepared statement has a name, it is only parsed once. After that, name will re-use the prepared statement regardless of what text is.

From the wiki:

(In node-postgres, if a new prepared statement is issued with the same name as an existent prepared statement, the existent prepared statement will be replaced and no error raised.)

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.

@whitelynx
Copy link
Contributor Author

Just to comment, the most useful thing from a user perspective would be to only prepare if either:

  • the named statement hasn't been prepared in this session yet
  • or, the named statement was prepared with different text

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 pg.Client itself. (and in pg.native, of course)

@brianc
Copy link
Owner

brianc commented May 3, 2013

👍 as long as it's tested it'll be happily accepted

@whitelynx
Copy link
Contributor Author

Just to clarify, which place accurately represents the current behavior? Either the docs or the FAQ should probably be corrected in the meantime.

@brianc
Copy link
Owner

brianc commented May 8, 2013

updated documentation

@brianc brianc closed this as completed Jul 3, 2013
@shalecraig
Copy link

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

@brianc
Copy link
Owner

brianc commented Aug 17, 2013

How is it alarming? What do you want me to clarify?

@markselby
Copy link

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;

@twistedvisions
Copy link

@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

If and only if name is provided within the config object does query result in a prepared statement.
If text and name are provided within the config, the query will result in the creation of a prepared statement.
If value and name provided within the config, the prepared statement will be executed. (Note: if the prepared statement takes no parameters, use value:[].)

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!

@twistedvisions
Copy link

Just as a further note, I identified whether my connections were being cached or not by changing the prepare function as so:

Query.prototype.prepare = function(connection) {
  var self = this;
  //prepared statements need sync to be called after each command
  //complete or when an error is encountered
  this.isPreparedStatement = true;
  //TODO refactor this poor encapsulation
  if(!this.hasBeenParsed(connection)) {
    connection.parse({
      text: self.text,
      name: self.name,
      types: self.types
    }, true);
    if(this.name) {
      connection.parsedStatements[this.name] = true;
    }
    console.log(self.name, "needed preparing")
  }
  else {
    console.log(self.name, "already prepared")
  }

@vitaly-t
Copy link
Contributor

vitaly-t commented May 3, 2016

@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 text based on whether it was the same connection as during the last call to get the object. This requires to be able to identify the current connection via some sort of unique id.

Question: Is there such thing within the current connection that can be used to uniquely identify the connection?

@vitaly-t
Copy link
Contributor

vitaly-t commented May 3, 2016

@brianc I think I got it figured out - client.secretKey contains that unique value that I need :)

@darkclouder
Copy link

You might want to update wiki/Prepared-Statements referring to this issue

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

6 participants