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

Setting connection runtime options #14

Closed
moll opened this Issue Jun 7, 2013 · 10 comments

Comments

Projects
None yet
2 participants
@moll
Collaborator

moll commented Jun 7, 2013

Hey,

I need to SET a few PostgreSQL options right after connecting. Where have you envisioned such settings to take place?

I see getRawConnection in postgres.js is reponsible for creating a new connection — that could be a place for a temporary injection, but what are your thoughts for long term?

@tgriesser

This comment has been minimized.

Owner

tgriesser commented Jun 7, 2013

Just to make sure I'm understanding the question - these are options that need to be set just after connection on each individual connection in the pool (before the connection is added to the pool) correct?

@moll

This comment has been minimized.

Collaborator

moll commented Jun 8, 2013

Yep, that's the way PostgreSQL runtime opts work: http://www.postgresql.org/docs/9.2/static/sql-set.html

tgriesser added a commit that referenced this issue Jun 9, 2013

Merge branch 'master' into gh-pages
* master:
  0.1.5
  Adding beforeCreate and afterCreate as options on the pool's config #14
  Allowing Knex.Raw statements in where clauses, #15
  shouldn't need bindings on raw queries
  minor code tweak
@tgriesser

This comment has been minimized.

Owner

tgriesser commented Jun 9, 2013

@moll - I added an option to specify a beforeCreate afterCreate function on the pool options during initialize, which accepts the connection that's being added to the pool, and a done function that should be called after any setup you need to do is complete.

Knex.Initialize({
  client: 'pg',
  connection: {...},
  pool: {
    afterCreate: function(conn, done) {
      conn.query('SET ....', function() {
         done();
      });
    }
  }
});

Since this is the actual connection - you'll need to use it like you would in the pg module.

Let me know if this works alright, or if there's anything that could be improved here.

@tgriesser tgriesser closed this Jun 9, 2013

@moll

This comment has been minimized.

Collaborator

moll commented Jun 9, 2013

Nice work.

Though, one thing, I think the naming for beforeCreate is a tad misleading — afterCreate would be more appropriate given it's actually called after creating the connection. Or if you were going for the "before adding to the pool" semantic, then beforeCreate doesn't fit either. But afterCreate would sound decent.

@moll

This comment has been minimized.

Collaborator

moll commented Jun 9, 2013

Oh and why not pass the error given to the callback by before/afterCreate up the stack? Though maybe that would require tearing down the connection before passing the error on.

@tgriesser

This comment has been minimized.

Owner

tgriesser commented Jun 9, 2013

Yeah, afterCreate sounds better... I was going for the "before adding to the pool"... but as you pointed out that doesn't quite fit.

I'm going to take a closer look at cleaning up the pool error handling in general, related to #4... but are you thinking that it should be something along the lines of:

conn.query('SET ....', function(err, resp) {
   done(err);
});
@moll

This comment has been minimized.

Collaborator

moll commented Jun 9, 2013

If you agree with afterCreate, do a quick rename before anyone starts using 0.1.5. ;)

@moll

This comment has been minimized.

Collaborator

moll commented Jun 9, 2013

By passing the error on, I meant instead of:

        this.beforeCreate(conn, function() {
          callback(null, conn);
        });

do

        this.beforeCreate(conn, function(err) {
          callback(err, conn);
        });
@tgriesser

This comment has been minimized.

Owner

tgriesser commented Jun 9, 2013

@moll - 0.1.6 is out... with both of the changes you suggested :)

@moll

This comment has been minimized.

Collaborator

moll commented Jun 9, 2013

Superb.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment