Commit 2ef6dc79 authored by Kevin Crum's avatar Kevin Crum Committed by Dian Fay
Browse files

feat!: preserve type when ordering by JSON fields 

BREAKING CHANGE: JSON fields were previously converted to text and sorted alphabetically.

closes issue #683
parent e934444a

lib/statement/order-by.js

+4 −2
Original line number Diff line number Diff line
@@ -30,13 +30,15 @@ exports = module.exports = (order, useBody) => { 


exports.fullAttribute = function (orderObj, useBody) {

  let field;

  const jsonAsText = !!orderObj.type; // Explicit casts must use as-text operators



  if (orderObj.expr) {

    field = orderObj.expr;

  } else if (useBody) {

    field = `body->>'${orderObj.field}'`;

    const operator = jsonAsText ? '->>' : '->';

    field = `body${operator}'${orderObj.field}'`;

  } else if (orderObj.field) {

    const parsed = parseKey(orderObj.field, () => {});

    const parsed = parseKey(orderObj.field, () => {}, jsonAsText);



    field = parsed.field;

  }

lib/util/parse-key.js

+8 −4
Original line number Diff line number Diff line
@@ -9,9 +9,10 @@ 
 * @module util/parseKey

 * @param  {String} key A reference to a database column. The field name may be quoted using double quotes to allow names which otherwise would not conform with database naming conventions. Optional components include, in order, [] and . notation to describe elements of a JSON field; ::type to describe a cast; and finally, an argument to the appendix function.

 * @param {Object} appendix A function which when invoked with an optional component of the key returns a value to be used later. So far used for operations (from {@linkcode where}) and ordering (from {@linkcode order}.

 * @param {Boolean} jsonAsText A boolean to determine which JSON extraction operators to use

 * @return {Object} An object describing the parsed key.

 */

exports = module.exports = function (key, appendix) {

exports = module.exports = function (key, appendix, jsonAsText = true) {

  key = key.trim();



  const jsonShape = [];       // describe a JSON path: true is a field, false an array index
@@ -44,6 +45,7 @@ exports = module.exports = function (key, appendix) { 
          // about type

          if (!hasCast) {

            hasCast = true;

            jsonAsText = true; // Explicit casts must use as-text operators



            buffer = parsed[parsed.push([]) - 1];

          }
@@ -97,17 +99,19 @@ exports = module.exports = function (key, appendix) { 
  if (jsonShape.length === 1) {

    elements.push(parsed.shift());



    const operator = jsonAsText ? '->>' : '->';

    if (jsonShape[0]) {

      // object key

      quotedField = `${quotedField}->>'${elements[0]}'`;

      quotedField = `${quotedField}${operator}'${elements[0]}'`;

    } else {

      // array index

      quotedField = `${quotedField}->>${elements[0]}`;

      quotedField = `${quotedField}${operator}${elements[0]}`;

    }

  } else if (jsonShape.length > 0) {

    elements = parsed.splice(0, jsonShape.length);



    quotedField = `${quotedField}#>>'{${elements.join(',')}}'`;

    const operator = jsonAsText ? '#>>' : '#>';

    quotedField = `${quotedField}${operator}'{${elements.join(',')}}'`;

  }



  if (hasCast) {

test/statement/order-by.js

+2 −2
Original line number Diff line number Diff line
@@ -67,7 +67,7 @@ describe('orderBy', function () { 
    assert.equal(orderBy([

      {field: 'col1', direction: 'asc', type: 'int'},

      {field: 'col2'}

    ], true), `ORDER BY (body->>'col1')::int ASC,body->>'col2' ASC`);

    ], true), `ORDER BY (body->>'col1')::int ASC,body->'col2' ASC`);

  });



  it('should ignore useBody with exprs', function () {
@@ -81,6 +81,6 @@ describe('orderBy', function () { 
      {field: 'jsonobj.element', direction: 'asc'},

      {field: 'jsonarray[1]', direction: 'desc'},

      {field: 'complex.element[0].with.nested.properties', direction: 'asc'}

    ]), `ORDER BY "jsonobj"->>'element' ASC,"jsonarray"->>1 DESC,"complex"#>>'{element,0,with,nested,properties}' ASC`);

    ]), `ORDER BY "jsonobj"->'element' ASC,"jsonarray"->1 DESC,"complex"#>'{element,0,with,nested,properties}' ASC`);

  });

});

test/util/parse-key.js

+28 −0
Original line number Diff line number Diff line
@@ -74,6 +74,34 @@ describe('parseKey', function () { 
      assert.equal(result.field, '"json"#>>\'{array,1,field,array,2}\'');

      assert.deepEqual(result.elements, ['array', '1', 'field', 'array', '2']);

    });



    it('should format a shallow JSON path with as-text off', function () {

      const result = parseKey('json.property', () => {}, false);

      assert.equal(result.rawField, 'json');

      assert.equal(result.field, '"json"->\'property\'');

      assert.deepEqual(result.elements, ['property']);

    });



    it('should format a JSON array path with as-text off', function () {

      const result = parseKey('json[123]', () => {}, false);

      assert.equal(result.rawField, 'json');

      assert.equal(result.field, '"json"->123');

      assert.deepEqual(result.elements, ['123']);

    });



    it('should format a deep JSON path with as-text off', function () {

      const result = parseKey('json.outer.inner', () => {}, false);

      assert.equal(result.rawField, 'json');

      assert.equal(result.field, '"json"#>\'{outer,inner}\'');

      assert.deepEqual(result.elements, ['outer', 'inner']);

    });



    it('should force as-text on if JSON has cast', function () {

      const result = parseKey('json.property::int', () => {}, false);

      assert.equal(result.rawField, 'json');

      assert.equal(result.field, '("json"->>\'property\')::int');

      assert.deepEqual(result.elements, ['property']);

    });

  });



  describe('operation appendices', function () {