Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

bind Buffer() parameters as binary instead of text #92

Closed
wants to merge 1 commit into from

3 participants

@nraynaud

Avoids having to encode blobs in queries.

@nraynaud nraynaud bind Buffer variables as binary values
Avoids having to encode blobs in query.
e0f6baa
@brianc
Owner

I appreciate the pull request - especially since it contains tests!

Let's discuss a bit before merging, just to be safe.

  • can you provide an integration test or two that actually round-trips to the database and back?
  • will this work with the native client as well as pure javascript?
  • do you think the same functionality could be achieved by monkey-patching at a higher level?

Reason I ask is mostly because I'd like to have the javascript & native bindings be drop in replacement for one another. To add more 'advanced' functionality like this though, the two would likely have to diverge. I know there are a few things you simply cannot do w/ the native libpq which you can do with the binary protocol such as intermixing binary & text parameters in the same query.

@nraynaud
  • I'll check your current integration tests, but I started developing in node.js 3 days ago, so I don't feel I can do fancy stuff
  • I didn't event check the native client, I'm trying to limit my exposure to C/C++
  • I guess I could monkey-patch the whole method, I don't see any other mean

Why don't you go full binary ? I don't exactly know what I'm talking about, so don't mind if this proposition is silly, but I suppose strings remain the same and the other stuff is just smaller.

I send zipped files to my DB, and I'll send other binary data soon, so it's a plus to have native binary.

@nraynaud

I see "paramFormats" that looks the same here :
http://www.postgresql.org/docs/9.1/static/libpq-exec.html

@dresende

What's the status of this?

@brianc
Owner

I believe the status is I'm still waiting to see if there can be any integration tests provided for this.

@dresende

A random buffer posting and then fetching wouldn't do as a test?

@brianc brianc closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 8, 2012
  1. @nraynaud

    bind Buffer variables as binary values

    nraynaud authored
    Avoids having to encode blobs in query.
This page is out of date. Refresh to see the latest.
Showing with 41 additions and 2 deletions.
  1. +14 −2 lib/connection.js
  2. +27 −0 test/unit/connection/outbound-sending-tests.js
View
16 lib/connection.js
@@ -143,15 +143,27 @@ p.bind = function(config, more) {
config.binary = config.binary || false;
var values = config.values || [];
var len = values.length;
+ var useBinary = false;
+ for (var j = 0; j < len; j++)
+ useBinary |= values[j] instanceof Buffer;
var buffer = this.writer
.addCString(config.portal)
.addCString(config.statement)
- .addInt16(0) //always use default text format
- .addInt16(len); //number of parameters
+ if (!useBinary)
+ buffer.addInt16(0);
+ else {
+ buffer.addInt16(len);
+ for (var j = 0; j < len; j++)
+ buffer.addInt16(values[j] instanceof Buffer);
+ }
+ buffer.addInt16(len); //number of parameters
for(var i = 0; i < len; i++) {
var val = values[i];
if(val === null || typeof val === "undefined") {
buffer.addInt32(-1);
+ } else if (val instanceof Buffer) {
+ buffer.addInt32(val.length);
+ buffer.add(val);
} else {
val = val.toString();
buffer.addInt32(Buffer.byteLength(val));
View
27 test/unit/connection/outbound-sending-tests.js
@@ -112,6 +112,33 @@ test('bind messages', function() {
.join(true, 'B');
assert.received(stream, expectedBuffer);
});
+
+ test('with named statement, portal, and buffer value', function() {
+ con.bind({
+ portal: 'bang',
+ statement: 'woo',
+ values: [1, 'hi', null, new Buffer('zing', 'UTF-8')]
+ });
+ var expectedBuffer = new BufferList()
+ .addCString('bang') //portal name
+ .addCString('woo') //statement name
+ .addInt16(4)//value count
+ .addInt16(0)//string
+ .addInt16(0)//string
+ .addInt16(0)//string
+ .addInt16(1)//binary
+ .addInt16(4)
+ .addInt32(1)
+ .add(Buffer("1"))
+ .addInt32(2)
+ .add(Buffer("hi"))
+ .addInt32(-1)
+ .addInt32(4)
+ .add(new Buffer('zing', 'UTF-8'))
+ .addInt16(0)
+ .join(true, 'B');
+ assert.received(stream, expectedBuffer);
+ });
});
Something went wrong with that request. Please try again.