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

Clarify node-pg's strategy for the JSON data type in Postgres #239

Merged
merged 8 commits into from Apr 22, 2013
Merged

Conversation

@brianc
Copy link
Owner

brianc commented Apr 22, 2013

Hi Brian,

Could you please mention what you think node-pg should be doing to handle the JSON data type that appeared in Postgres 9.2?

I've asked for more info here previously:

#208 (comment)

It'd be ideal if node-pg could parse or stringify the value of the json column going out or coming into postgres. If you need a hand with it let us know (although I also said that about node-sql and we've been super busy). In the interim we need to form a strategy for handling json data using node-pg.

Cheers!
Nick

@brianc
Copy link
Owner

brianc commented Jan 14, 2013

I would like to support it fully. I'm not on postgres 9.2 on any of my apps yet -- would you be able to provide a simple script example? I can work that into a failing test & then passing test and then, boom, feature.

@nicholasf
Copy link
Author

nicholasf commented Jan 14, 2013

Will do.

@brianc
Copy link
Owner

brianc commented Feb 6, 2013

Once travis-ci gets 9.2 I'll roll out support for using JSON.parse and JSON.stringify for the json OID.

@spollack
Copy link
Contributor

spollack commented Mar 22, 2013

+1 for this feature.

@brianc
Copy link
Owner

brianc commented Mar 22, 2013

I totally agree. Anyone know off the top of their head the status on pg @ 9.2 on travis-ci?

@spollack
Copy link
Contributor

spollack commented Mar 22, 2013

I do not, sorry.

On Mar 22, 2013, at 9:45 AM, Brian C notifications@github.com wrote:

I totally agree. Anyone know off the top of their head the status on pg @ 9.2 on travis-ci?


Reply to this email directly or view it on GitHub.

This was referenced Apr 22, 2013
@brianc
Copy link
Owner

brianc commented Apr 22, 2013

Now that we've got Postgres9.2 on travis I can work on adding in some tests and support for the JSON oids. They'll only run if SELECT version() returns >= 9.2

@spollack
Copy link
Contributor

spollack commented Apr 22, 2013

fantastic, thanks @brianc !

brianc added 2 commits Apr 22, 2013
requires >= 9.2 of postgres
Forgot to add this to the last commit
@brianc
Copy link
Owner

brianc commented Apr 22, 2013

Uhhh I kinda jacked this up by branching off another feature branch but it should merge cleanly since it was the last thing in the history. git pwns us all.

@brianc
Copy link
Owner

brianc commented Apr 22, 2013

Tests passing.

Postgres @ 9.1 (travis's default version)
https://travis-ci.org/brianc/node-postgres/jobs/6539359#L182

Postgres @ 9.2
https://travis-ci.org/brianc/node-postgres/jobs/6539359#L430

💃 🎉

brianc added a commit that referenced this pull request Apr 22, 2013
add support for json data type
@brianc brianc merged commit 1e888da into master Apr 22, 2013
1 check passed
1 check passed
default The Travis build passed
Details
@brianc brianc deleted the json-support branch Apr 22, 2013
@brianc
Copy link
Owner

brianc commented Apr 22, 2013

@nicholasf npm install pg @ v1.1.0 for built in json support.

@kunanyi
Copy link

kunanyi commented Apr 29, 2013

Apologies if I'm in the wrong spot here, but do we need the "contains" (@>) operator to also work with this?

Have put an example in the open hstore parser issue.

Not sure if this is a general parsing issue, or something specific to json/hstore?

@brianc
Copy link
Owner

brianc commented Apr 29, 2013

Everything in the query text gets passed directly to the backend PostgreSQL server without any manipulation what so ever by node or node-postgres apart from Buffer("YOUR QUERY TEXT HERE").toString('utf8') so if you're getting a syntax error from the back end 99% likely it's something wrong with your query.

@kunanyi
Copy link

kunanyi commented Apr 30, 2013

Think the issue I'm having is with prepared statements and hstore under pg v1.1.0:

Running this...

db.query('SELECT * FROM stuff WHERE data @> $1::hstore;', [{x:1}]);

.... raises the following entry in my Postgres.app log....

ERROR:  Unexpected end of string
STATEMENT:  SELECT * FROM stuff WHERE data @> $1::hstore;

... hinting that $1 is not being expanded?

Prepared statements with hstore parameters do work OK inside PLSQL :

CREATE TABLE stuff(id SERIAL PRIMARY KEY, data HSTORE);
INSERT INTO stuff (data) VALUES ('x=>"1", y=>10'::hstore);
PREPARE test (hstore) AS SELECT * FROM stuff WHERE data @> $1::hstore;
EXECUTE test('x=>1');

=> 1 row

Somewhere along the way the JSON is not being accepted as a parameter by postgres? (probably something I'm doing wrong, but could someone confirm the above code and schema work from them?)

Thanks for any ideas, and a great library 👍

@kunanyi
Copy link

kunanyi commented Apr 30, 2013

The issue is I am confusing hstore with JSON :) [slaps forehead]

Was hoping that Postgres would provide a converter, but no luck. Maybe 9.3 will have something.

In the mean time, wrote a little JS function:

var json_to_hstore = function(json) {
  var a = [], i = 0, k;
  for (k in json) {
    if (json.hasOwnProperty(k)) { 
      a[i] = '"' + k + '"=>"' + encodeURIComponent(JSON.stringify(json[k])) + '"';
      i++;
    }   
  }
  return a.join(', ');
};

Which might help someone in this way:

s = 'SELECT * FROM stuff WHERE data @> $1::hstore;'
db.query(s, [json_to_hstore({x:1})]);
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.