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

Rough implementation of PostgreSQL JSON support #2314

Merged
merged 14 commits into from Sep 24, 2014

Conversation

9 participants
@sveinnfannar
Contributor

sveinnfannar commented Sep 20, 2014

This is a rough implementation of the JSON datatype in Postgres 9.3 but can easily be extended for JSONB in 9.4.

The query interface needs to be discussed further (see #1016). In this implementation both the where and attributes options work for finders. Currently, querying JSON data-types uses the same query language as Postgres without providing any abstractions.

Things yet to do:

  • Discuss API further and update implementation accordingly
  • Update documentation
  • Implement JSONB from PostgreSQL 9.4

Here a simple example of querying:

Content.find({ where: "metadata->>'language' = 'English'" })
  .then(function(content) {
    console.log('Result:', content.dataValues);
  })

A more thorough example can be found under examples/postgresql-json-support.

@Bondifrench

This comment has been minimized.

Bondifrench commented Sep 20, 2014

+1 Awesome stuff

@hph

This comment has been minimized.

hph commented Sep 21, 2014

👍

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Sep 22, 2014

I was thinking about doing something like:

where: sequelize.or(
  sequelize.json("metadata->>'language'", 'english'),
  sequelize.json("metadata->>'language'", 'danish')
)

The API is tricky on this one, i've been thinking about implementing https://github.com/goodybag/mongo-sql for something like this.

@sveinnfannar

This comment has been minimized.

Contributor

sveinnfannar commented Sep 22, 2014

I like having a sequelize.json function for this. What part of MoSQL are you thinking were appropriate for this?

Maybe we should even abstract away the postgres json syntax, something like this:

where: sequelize.or(
    sequelize.json({ metadata: { language: 'icelandic' } }),
    sequelize.json({ metadata: { language: 'danish' } })
)

That way we can also free users from dealing with -> vs. ->>.

@janmeier

This comment has been minimized.

Member

janmeier commented Sep 22, 2014

That syntax indeed looks promising @sveinnfannar.

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Sep 22, 2014

@sveinnfannar MoSQL has some JSON syntax IIIRC.

Isn't there difference between stuff like -> and ->> etc though?

@janmeier

This comment has been minimized.

Member

janmeier commented Sep 22, 2014

Seems double arrow is "get as text". Not sure how usefull that is in sequelize context http://www.postgresql.org/docs/9.3/static/functions-json.html

@sveinnfannar

This comment has been minimized.

Contributor

sveinnfannar commented Sep 22, 2014

Yes @janmeier that's true. -> returns a json value while ->> returns a string.

What bugs me is this:
When I execute select * from content where metadata->'language' = 'icelandic'; I get: ERROR: operator does not exist: json = unknown

While select * from content where metadata->>'language' = 'icelandic'; works as expected.

Same when drilling down into nested objects. select * from content where metadata->>'pg_rating'->>'denmark' = 'G'; gives ERROR: operator does not exist: text ->> unknown.
So this is the correct way to drill down: select * from content where metadata->'pg_rating'->>'denmark' = 'G';

Maybe this doesn't matter and I'm just nitpicking :)
But I would still argue that there is value in abstracting the postgres syntax to be able to support other databases with nested data-types later on.

EDIT: I just figured out I can just use the #>> operator to avoid this.

@sveinnfannar

This comment has been minimized.

Contributor

sveinnfannar commented Sep 23, 2014

I've implemented the where condition syntax discussed above.

So the this:

Content.find({
  where: {
    metadata: {
      language: 'icelandic',
      pg_rating: { 'dk': 'G' }
    },
    another_json_field: { x: 1 }
  }
})

yields something similar to this:

SELECT * FROM "Content" WHERE metadata#>>'{language}' = 'icelandic' and metadata#>>'{pg_rating,dk}' = 'G' and another_json_field#>>'{x}' = '1'

Check it out and tell me what you think.

I also wanted to get some input on where it would be most appropriate to have this totally postgres specific piece of code.

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Sep 23, 2014

metadata#>>'{pg_rating,dk}' what's significant about this syntax compared to metadata->>pg_rating->dk?

Edit: Sorry, just read the comment above. It seems weird that the PG syntax is so strict about that. When would you use what operator?

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Sep 23, 2014

It would ideal to also support a dot notation syntax.

sequelize.where(sequelize.json('some.nested.structure'), 'value')
{'some.nested.structure': 'value'}

Option two might be too far fetched since we need to see if there's a '.' and then see if the first elem matches a json attribute.

@sveinnfannar

This comment has been minimized.

Contributor

sveinnfannar commented Sep 23, 2014

Yes, I agree.
What is the difference between using the sequelize.where function and passing in an object with conditions { where: { .. } }?

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Sep 23, 2014

@sveinnfannar there isn't one, necessarily, depends a little on the context. where() might be more pleasing to the eye when using in conjuction with .and(). It was only to support dot notation, which might be unwieldy in regular where.

@janmeier

This comment has been minimized.

Member

janmeier commented Sep 23, 2014

sequelize.where allows you to use an object as a key. In mick's example the key is an instance of sequelize.json, you can't put that into a POJO ie:

{
  sequelize.json('some.nested.structure'): 'value'
}

Baaad syntax

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Sep 23, 2014

Mind updating travis.yml with:

addons:
  postgresql: "9.3"

Per http://docs.travis-ci.com/user/using-postgresql/ so that the tests can actually run on travis ;p

@sveinnfannar

This comment has been minimized.

Contributor

sveinnfannar commented Sep 23, 2014

Thanks guys, that makes perfect sense.
I'll commit the changes later tonight.

@sveinnfannar

This comment has been minimized.

Contributor

sveinnfannar commented Sep 23, 2014

I added the dot syntax and an optional value argument. So this is now possible sequelize.json('metadata.language', 'danish').

Unfortunately I wasn't able to figure out how to use where like you talked about, but I was probably using it wrong.

Example:
Content.find({ where: sequelize.where(sequelize.json('some.nested.structure'), 'value') })

Possibly unhandled TypeError: Cannot read property 'name' of undefined
    at Object.module.exports.QueryGenerator.getWhereConditions (/Users/sveinnfannar/Dev/sequelize/lib/dialects/abstract/query-generator.js:1245:55)
    at Object.module.exports.QueryGenerator.selectQuery (/Users/sveinnfannar/Dev/sequelize/lib/dialects/abstract/query-generator.js:1040:30)
...
@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Sep 24, 2014

where likely needs to be wrapped in an and right now.
Is where: sequelize.json('some.nested.structure', 'value') also possible?

Great work by the way @sveinnfannar

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Sep 24, 2014

I think we're missing a test using where with sequelize.json and dot notation syntax.

@sveinnfannar

This comment has been minimized.

Contributor

sveinnfannar commented Sep 24, 2014

@mickhansen, yes thats now possible.
I wanted to test Utils.json in isolation so the tests can be found in test/utils.test.js.

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Sep 24, 2014

@sveinnfannar yes i understand that, it would just be great to also have a test covering where: sequelize.json('dot.notation') so we don't have any possible regressions. If it's asking too much let me know :)

@sveinnfannar

This comment has been minimized.

Contributor

sveinnfannar commented Sep 24, 2014

No of course not :)
Thanks for the all the feedback btw.

What are the next steps then? What do you think needs to be done before I can update the documentation and we can merge?
I guess I'll add the BJSON support in another pull request.

mickhansen added a commit that referenced this pull request Sep 24, 2014

Merge pull request #2314 from sveinnfannar/feature/postgresql-json-su…
…pport

Rough implementation of PostgreSQL JSON support

@mickhansen mickhansen merged commit 2965c04 into sequelize:master Sep 24, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Sep 24, 2014

Merging now, so not a lot more.
BJSON support needs some API discussing in how we should do partial updates, indexes, etc. Also Travis doesn't support 9.4 yet.

@sveinnfannar

This comment has been minimized.

Contributor

sveinnfannar commented Sep 24, 2014

Great, thanks for all the help.

@sveinnfannar sveinnfannar deleted the sveinnfannar:feature/postgresql-json-support branch Sep 24, 2014

@sveinnfannar sveinnfannar restored the sveinnfannar:feature/postgresql-json-support branch Sep 25, 2014

@vvo

This comment has been minimized.

Contributor

vvo commented Oct 1, 2014

awesome work!

@benedictfischer09

This comment has been minimized.

benedictfischer09 commented Oct 3, 2014

a nice feature would be the ability to automatically wrap portions of the json response in a separate model

  Content.find({ 
      where: "metadata->>'language' = 'English'" 
      wrap: {metadata: {language: Language}}
    })
  .then(function(content) {
    if (content.language.isEuropean()){
      console.log('do something');
    } else {
      console.log('do something else');
    }
  })

Also useful for collections

  Content.find({ 
      where: "metadata->>'language' = 'English'" 
      wrap: {metadata: {addresses: Address}}
    })
  .then(function(content) {
    for(var i = content.metadata.addresses.length - 1; i >= 0; i--){
    var address = content.metadata.addresses[i];
     if (address.isIn('U.S.A')){
       console.log('do something')
     }
  })

The mongoose ODM implements something like this. It lets you specify that a subdocument maps to a model and any query functions take care of wrapping response data in instances of that model but it might be more flexible to allow the mapping to occur in the query
http://mongoosejs.com/docs/subdocs.html

If people think its a good idea I would be interested in pairing with someone to implement it, I've never contributed to a open source project like this so it would be cool to work with someone who has more experience

@scottpersinger

This comment has been minimized.

scottpersinger commented Nov 3, 2014

How does this compare with the Mongo query DSL?

http://docs.mongodb.org/manual/tutorial/query-documents/?_ga=1.228937756.1454693923.1415038091

Is it worth re-using their syntax or looking to it for inspiration?

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Nov 3, 2014

@scottpersinger i've actually been looking at implementing a lot of the $ keys for regular querying, they make sense for json aswell (json supporting nested structures).

@G3z

This comment has been minimized.

G3z commented Dec 29, 2014

hello, what is the best way to build a query like this?

SELECT * FROM "users" WHERE (profile #>> '{name}') ILIKE '%word%'

i was trying something like this but i can't get the right query

User.all({
    where: Sequelize.json({
        profile: {
            name: {
                ilike: "%"+value+"%"
            }
        }
    })
});

while this works:

User.all({
    where: ["(profile #>> '{name}') ILIKE ?", "%"+value+"%"]
});
@Bondifrench

This comment has been minimized.

Bondifrench commented Dec 29, 2014

Have a look at one of the examples:
https://github.com/sequelize/sequelize/blob/master/examples/postgres-json-datatype/app.js

It seems you need to use Sequelize.json in your where clause.

@G3z

This comment has been minimized.

G3z commented Dec 29, 2014

Actually i am (updated code above removing my own wrapper)
thing is i don't see a way to make
Content.find({ where: Sequelize.json({ metadata: { language: 'Japanese' } }) })
https://github.com/sequelize/sequelize/blob/master/examples/postgres-json-datatype/app.js#L56
use LIKE or ILIKE

@mickhansen

This comment has been minimized.

Contributor

mickhansen commented Dec 30, 2014

Not sure JSON supports anything other than equality right now having gone over the source code.

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