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

Lack of robust json/jsonb support #5078

Closed
jackwanders opened this issue Dec 9, 2015 · 11 comments
Closed

Lack of robust json/jsonb support #5078

jackwanders opened this issue Dec 9, 2015 · 11 comments

Comments

@jackwanders
Copy link

I am attempting to develop a sails project with sails-postgresql. One of my tables will need to contain a column with type jsonb, on which I will be creating indexes on nested properties. For example, the following is supported in PostgreSQL 9.4:

CREATE TABLE myModels (
    id bigserial PRIMARY KEY,
    name varchar(255) not null,
    data jsonb not null
);
CREATE UNIQUE INDEX name_dataId ON files (name, (data->>'id'));
CREATE INDEX data_id ON files ((data->>'id'));

There seems to be a lack of support in sails-postgresql for this type of functionality; in particular, 'jsonb' as a valid attribute type in sails model definitions when using a sails-postgresql connection, and the ability to query on json property values.

For example, I would expect the following valid PSQL:

SELECT * FROM myModels WHERE data->>'id' = 1234;

to map to a Waterline call similar to one of the following:

MyModel.find({
  "data->>'id'": 1234 // mimics the query language, data->>'id' being a indexable, searchable value
})
//////////////////
MyModel.find({
  data: {
    id: 1234 // mimics the structure of data within the table, relies of sparse matching
  }
})

Last June, https://github.com/balderdashy/sails-postgresql/issues/21 was closed with the statement that, because Waterline didn't support querying within documents, then sails-postgresql wouldn't either. Also, the claim that a user should modify their data persistence strategy based on the feature set of the software they're using doesn't necessarily fly with me. PostgreSQL supports these features natively, so to me, it seems to follow that it would be beneficial for a PostgreSQL adapter to support those features as well.

Last August, an issue was opened against Waterline itself (https://github.com/balderdashy/waterline/issues/562), but it was recommended that the issue be brought here.

Another issue was opened more recently (https://github.com/balderdashy/sails-postgresql/issues/187), but was closed due to inactivity. I would be happy to contribute to this end, assuming that other core contributors and maintainers believe this is functionality that is possible within sails-postgresql.

@jackwanders
Copy link
Author

For what it's worth, part of the issue might be the dependence of sails-postgresql on waterline-sequel, which, not unexpectedly, is unaware of how to work with json fields and queries.

When I debug to find the actual PSQL query being executed against my database, for the above example I get:

SELECT "mymodel"."name", "mymodel"."data" FROM "mymodel" AS "mymodel" WHERE "mymodel"."data->>'id'" = '1234

when I run this query in the psql command line tool, I get the same error that I get from waterline:

error: column mymodel.data->>'id' does not exist

On a whim, I decided to hack at https://github.com/balderdashy/waterline-sequel/blob/master/sequel/lib/criteriaProcessor.js to make it aware of the PSQL json operator @>, as documented here: http://www.postgresql.org/docs/9.4/static/functions-json.html. In the swtich statement that acts on the various operator values, I added:

case '@>':

      if(this.parameterized) {
        this.values.push(value);
        str = '@> ' + '$' + this.paramCount;
      }
      else {
        if(_.isString(value) && !escapedDate) {
          value = '"' + utils.escapeString(value) + '"';
        }
        str = '@> ' + value;
      }

      break;

I then created a Waterline find call like so:

MyModel.findOne({
  'data': {
    '@>': JSON.stringify({id: myDesiredDataId})
  }
})

This call resulted in the query:

SELECT "mymodel"."name", "mymodel"."data" FROM "mymodel" AS "mymodel"  WHERE "mymodel"."data" @> '{"uploadId":"6582e314-f0d3-41e0-8ec0-245ad394dfe3~110"}' LIMIT 1

This returned the expected model in the .exec() callback, and all was well.

This leads me to believe that there may be value in a waterline-postgres type package that does exactly what waterline-sequel does, but is augmented to provide support for JSON related queries and functions.

@devinivy
Copy link

Interesting! @tjwebb how might this fly with your knex-based adapters? Does knex do a good job with this? This issue is also related to broader waterline features around deep population and association denormalization.

@tjwebb
Copy link
Contributor

tjwebb commented Dec 14, 2015

There's better jsonb type support here: https://github.com/waterlinejs/postgresql-adapter

I don't support json queries yet, though

@jackwanders
Copy link
Author

@tjwebb, are there any plans to support any of the operators found here: http://www.postgresql.org/docs/9.4/static/functions-json.html ?

@tjwebb
Copy link
Contributor

tjwebb commented Dec 16, 2015

@jackwanders yes I'd love to support all json operations, I just haven't gotten to it yet. Paying the bills and so forth :/ Any help is appreciated.

@jackwanders
Copy link
Author

That's good to hear, @tjwebb. It seems like a primary question would be how to best map the JSON operators to the existing Waterline Query Language contract. Some operators in Table 9-41 are just where clause operators and seem relatively straightforward to implement, relying only on the ability to detect the data type of the value provided to a 'contains' criteria modifier (@>, ?, and ?|). Some seem to require new criteria modifiers (e.g. <@ is something like a 'within' modifier).

I'm curious, does Waterline, in general, support the notion of either allowing existing criteria modifiers to accept new data types (as in the case of 'contains' accepting an object or array), or creating brand new modifiers (such as 'within'), and if so, where would that implementation occur? It seems that postgresql-adapter still has a dependency on waterline-sequel to process query criteria, so I'm not sure if a new criteria processor would be required, or if postgresql-adapter would need to massage the data passed to the criteria processor in order to get the right query back out.

@devinivy
Copy link

@jackwanders the way we've begun thinking about database-specific functionality is in terms of "interfaces" and "features". Adding to waterline's query language is difficult because official adapters need to keep up to date. So typically these decisions need to be made deliberately and carefully. However, if an adapter should have some DB-specific functionality, it may be spec'd and tested as a "feature." The waterline adapter tests already are organized to deal with features.

Implementing "within" on the current adapters would require changes to the adapters themselves (most likely) and waterline-sequel. I believe waterline itself ignores criteria modifiers it doesn't understand, and normalizes the ones it does understand.

@DaAwesomeP
Copy link

Postgre's JSON integration is extremely powerful. Using it alongside with HSTORE allows for it to effectively act as a document-based database with both document and key-value functionality. However, it's keeps that speed that Postgre always has.

I think that if you treat it separately from SQL and more like Mongo or CouchDB where the SQL is simply the request to get to the data, then it may be easier to integrate. It might be alright to even have a separate module for it.

@sailsbot
Copy link

Thanks for posting, @jackwanders. I'm a repo bot-- nice to meet you!

It has been 30 days since there have been any updates or new comments on this page. If this issue has been resolved, feel free to disregard the rest of this message. On the other hand, if you are still waiting on a patch, please:

  • review our contribution guide to make sure this submission meets our criteria (only verified bugs with documented features, please; no questions, commentary, or bug reports about undocumented features or unofficial plugins)
  • create a new issue with the latest information, including updated version details with error messages, failing tests, and a link back to the original issue. This allows GitHub to automatically create a back-reference for future visitors arriving from search engines.

Thanks so much for your help!

@particlebanana
Copy link
Contributor

I added this to the backlog of the Waterline repo as it requires essentially modifying the criteria language.

In the future feature requests like this will go through the updated Contribution Guide to make their way into roadmap.

@johnabrams7 johnabrams7 transferred this issue from balderdashy/sails-postgresql Apr 30, 2019
@alxndrsn
Copy link

The waterline repo's backlog refers to this issue in the Pending Proposals section. If the proposal is still pending, perhaps this issue should be re-opened; if not then should the entry in the Pending Proposals section should be removed?

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

No branches or pull requests

7 participants