Premade query object #228

Closed
wants to merge 3 commits into from

3 participants

@grncdr

Over on #227 Brian said:

If you need contributions on anything in particular I'd love to help you out.

This change would make it possible for any-db to remove the adapter layer for postgres and expose (patched) Client objects directly. The change is motivated by the way ConnectionPool.query works. The pool is able to return a QueryAdapter synchronously because connection.query accepts it as an optional parameter.

In this patch I've added support to the pure-js driver for separating initialization and construction in Query objects, which enables Client.query) to use a query_object from the config. If the general idea makes sense to you I'll try and do the same for the NativeQuery object.

@grncdr grncdr referenced this pull request Dec 10, 2012
Closed

The connection pool sucks #227

@brianc
Owner

is this more of a code-quality thing? Basically marking the native result property as "private" through the leading underscore convention for private member variables?

Sort of, I'm not actually too bothered by whether it has the leading underscore or not, but the inconsistency between this and the pure-js Query object (which uses the underscore) means I have to detect which one is in use in any-db. Right now (in my local copy) I just have a var result = query.result || query._result and that works for me, but I figured if I was modifying this code anyways I may as well fix it up. The larger change is having the 'end' event emit the result object, with that in place I can completely ignore whatever it's named on the query object.

@grncdr

So I just noticed the #171 accomplishes the same goal, and is a much smaller change. I like the approach @troyk used more than this one, so if you don't mind terribly I'm going to redo this patch by rebasing his previous PR

@troyk

@grncdr if I recall, there was a test that needed to be rewritten not to use the same query object repeatedly and I just got busy with life and such

@brianc
Owner

yeah @troyk I'm working on getting those tests passing right now. Sorry for the very belated pull request merge. I too was busy w/ life & such. 😄 Merging in a few minutes.

@grncdr

I guess I'll wait before opening this next pull request then ;)

@brianc
Owner

taking longer than expected to merge. there's a bunch of hackery in the binary protocol support I'm undoing.

@grncdr grncdr referenced this pull request Dec 11, 2012
Closed

Normalize query config #229

@grncdr

Closing this as #171 got merged and #229 adds the same functionality for the native bindings.

@grncdr grncdr closed this Dec 11, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment