Navigation Menu

Skip to content

Commit

Permalink
fix(abstract): patch jsonb operator for pg if value is json (#13780)
Browse files Browse the repository at this point in the history
* fix(abstract): dialog query-generator for JSON in PG

Added check to traverseJSON that checks if value is JSON and passes
result to jsonPathExtrationQuery

Added isJson param to jsonPathExtractionQuery

Updated postgres JSON join arrow to change if value is JSON

 with '#' will be ignored, and an empty message aborts the commit.

* fix(abstract): updated unit tests

Added check if value is string and operator is contains

Updated unit tests with new expected outputs

* fix(abstract): add query generator unit tests

Added tests to validate jsonPathExtractionQuery works with and without
isJSON

* fix(abstract): Fix merge error

Removed extra bracket from merge conflict

* fix(abstract): Patch failing tests

Updated query generator tests to handle all supported DB types

* fix(tests): update test name

Co-authored-by: Sascha Depold <sdepold@users.noreply.github.com>
  • Loading branch information
Drethic and sdepold committed Dec 22, 2021
1 parent b532ab1 commit a2375c5
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 8 deletions.
24 changes: 18 additions & 6 deletions lib/dialects/abstract/query-generator.js
Expand Up @@ -1095,12 +1095,13 @@ class QueryGenerator {
/**
* Generates an SQL query that extract JSON property of given path.
*
* @param {string} column The JSON column
* @param {string|Array<string>} [path] The path to extract (optional)
* @returns {string} The generated sql query
* @param {string} column The JSON column
* @param {string|Array<string>} [path] The path to extract (optional)
* @param {boolean} [isJson] The value is JSON use alt symbols (optional)
* @returns {string} The generated sql query
* @private
*/
jsonPathExtractionQuery(column, path) {
jsonPathExtractionQuery(column, path, isJson) {
let paths = _.toPath(path);
let pathStr;
const quotedColumn = this.isIdentifierQuoted(column)
Expand Down Expand Up @@ -1135,8 +1136,9 @@ class QueryGenerator {
return `json_unquote(json_extract(${quotedColumn},${pathStr}))`;

case 'postgres':
const join = isJson ? '#>' : '#>>';
pathStr = this.escape(`{${paths.join(',')}}`);
return `(${quotedColumn}#>>${pathStr})`;
return `(${quotedColumn}${join}${pathStr})`;

default:
throw new Error(`Unsupported ${this.dialect} for JSON operations`);
Expand Down Expand Up @@ -2484,11 +2486,21 @@ class QueryGenerator {
path[path.length - 1] = tmp[0];
}

const pathKey = this.jsonPathExtractionQuery(baseKey, path);
let pathKey = this.jsonPathExtractionQuery(baseKey, path);

if (_.isPlainObject(item)) {
Utils.getOperators(item).forEach(op => {
const value = this._toJSONValue(item[op]);
let isJson = false;
if (typeof value === 'string' && op === Op.contains) {
try {
JSON.stringify(value);
isJson = true;
} catch (e) {
// failed to parse, is not json so isJson remains false
}
}
pathKey = this.jsonPathExtractionQuery(baseKey, path, isJson);
items.push(this.whereItemQuery(this._castKey(pathKey, value, cast), { [op]: value }));
});
_.forOwn(item, (value, itemProp) => {
Expand Down
54 changes: 52 additions & 2 deletions test/unit/dialects/abstract/query-generator.test.js
Expand Up @@ -2,8 +2,9 @@

const chai = require('chai'),
expect = chai.expect,
Op = require('sequelize/lib/operators'),
getAbstractQueryGenerator = require('../../support').getAbstractQueryGenerator;
Op = require('../../../../lib/operators'),
Support = require('../../support'),
getAbstractQueryGenerator = Support.getAbstractQueryGenerator;
const AbstractQueryGenerator = require('sequelize/lib/dialects/abstract/query-generator');

describe('QueryGenerator', () => {
Expand Down Expand Up @@ -134,6 +135,55 @@ describe('QueryGenerator', () => {
});
});

describe('jsonPathExtractionQuery', () => {
const expectQueryGenerator = (query, assertions) => {
const expectation = assertions[Support.sequelize.dialect.name];
if (!expectation) {
throw new Error(`Undefined expectation for "${Support.sequelize.dialect.name}"!`);
}
return expectation(query);
};

it('should handle isJson parameter true', function() {
const QG = getAbstractQueryGenerator(this.sequelize);
expectQueryGenerator(() => QG.jsonPathExtractionQuery('profile', 'id', true), {
postgres: query => expect(query()).to.equal('(profile#>\'{id}\')'),
sqlite: query => expect(query()).to.equal('json_extract(profile,\'$.id\')'),
mariadb: query => expect(query()).to.equal('json_unquote(json_extract(profile,\'$.id\'))'),
mysql: query => expect(query()).to.equal("json_unquote(json_extract(profile,'$.\\\"id\\\"'))"),
mssql: query => expect(query).to.throw(Error),
snowflake: query => expect(query).to.throw(Error),
db2: query => expect(query).to.throw(Error)
});
});

it('should use default handling if isJson is false', function() {
const QG = getAbstractQueryGenerator(this.sequelize);
expectQueryGenerator(() => QG.jsonPathExtractionQuery('profile', 'id', false), {
postgres: query => expect(query()).to.equal('(profile#>>\'{id}\')'),
sqlite: query => expect(query()).to.equal('json_extract(profile,\'$.id\')'),
mariadb: query => expect(query()).to.equal('json_unquote(json_extract(profile,\'$.id\'))'),
mysql: query => expect(query()).to.equal("json_unquote(json_extract(profile,'$.\\\"id\\\"'))"),
mssql: query => expect(query).to.throw(Error),
snowflake: query => expect(query).to.throw(Error),
db2: query => expect(query).to.throw(Error)
});
});

it('Should use default handling if isJson is not passed', function() {
const QG = getAbstractQueryGenerator(this.sequelize);
expectQueryGenerator(() => QG.jsonPathExtractionQuery('profile', 'id'), {
postgres: query => expect(query()).to.equal('(profile#>>\'{id}\')'),
sqlite: query => expect(query()).to.equal('json_extract(profile,\'$.id\')'),
mariadb: query => expect(query()).to.equal('json_unquote(json_extract(profile,\'$.id\'))'),
mysql: query => expect(query()).to.equal("json_unquote(json_extract(profile,'$.\\\"id\\\"'))"),
mssql: query => expect(query).to.throw(Error),
snowflake: query => expect(query).to.throw(Error),
db2: query => expect(query).to.throw(Error)
});
});
});

describe('queryIdentifier', () => {
it('should throw an error if call base quoteIdentifier', function() {
const QG = new AbstractQueryGenerator({ sequelize: this.sequelize, _dialect: this.sequelize.dialect });
Expand Down

0 comments on commit a2375c5

Please sign in to comment.