refactoring the native "connection" code #257

Closed
booo opened this Issue Jan 24, 2013 · 3 comments

Comments

2 participants
Collaborator

booo commented Jan 24, 2013

What do you think about refactoring the native binding aka "connection" code? I would like to create a native connection class that is more or less like the javascript connection "class". It should implement the same public functions and emit the same class of events. I'm not sure if this is a super easy undertaking but I think it's useful and fun to do. If you agree I would start pushing to branch and inform you about the progress. Would love to get some feedback, thoughts about it.

The interface of the javascript class:

events:

  • connect
  • sslconnect
  • authenticationCleartextPassword
  • authenticationMD5Password
  • backendKeyData
  • readyForQuery
  • rowDescription
  • dataRow
  • portalSuspended
  • commandComplete
  • copyInResponse
  • copyOutResponse
  • copyData
  • notification
  • error
  • notice

functions:

  • connect(port, host)
  • startup(config)
    • { user: "foo", database: "bar }
  • requestSsl()
  • password(password)
  • sync()
  • canncel(processID, secretKey)
  • end()
Owner

brianc commented Jan 24, 2013

That would be pretty sweet, the api parity would be welcome as the sugar provided by the Client and Query objects could be totally reused, and you're totally welcome to try it. I would caution though it might be harder than it initially looks unless you intend on 'faking' some of the events.

AFAIK libpq parses the postgres protocol much like the pure JavaScript bindings and does some aggregation/hiding of events. For example: portalSuspended & the authentication methods are somewhat 'hidden' underneath the api libpq presents. It's been a while since I looked at the API for libpq so things might have changed since then.

When I set out to do the bindings initially I took the path of least resistance. My thinking was "the less code there is, the less likely for memory leaks or hard to track down native bugs." That doesn't mean my approach was best by any means, but that is why the code is a lot more "get the data into JavaScript land as fast as possible and then create API parity within JavaScript." There's also the slight performance hit when crossing from the pure JavaScript to v8 C++ layer so I tried to keep that to a minimum.

The current JavaScript Client and Query objects rely on a rely on a reference to the JavaScript connection object. It wouldn't be too hard to test by just passing in your connection as another 'run' of the integration suite...so that's a win.

Collaborator

booo commented Jan 26, 2013

I moved the connection logic from the client to the connection. Branch is here:

https://github.com/booo/node-postgres/compare/native_connection

Does this make sense to you?

Next step is the implementation of prepared statements etc. in the new nativeConnection mdoule. Involved methods are:

  • query(text)
  • execute({})
  • flush()
  • parse({})
  • bind({})
  • describe({})
  • some copy/streaming functions related to the connection :/

Maybe we want to hide this process as well in the javscriptConnection module...

Owner

brianc commented Oct 20, 2014

I refactored the hell out of the native connection code! libq and pg-native

@brianc brianc closed this Oct 20, 2014

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