Commit 29e29908 authored by Dian Fay's avatar Dian Fay
Browse files

refactor!: don't add an ORDER BY clause unless explicitly called for 

BREAKING CHANGE: results used to be implicitly ordered by the relation's primary key, or otherwise by the first column. This is no longer the case; ordering must be specified if it is desired.
parent 4d657e1b

lib/statement/select.js

+8 −32
Changes for lib/statement/select.js: 8 added lines, 32 removed lines.
Original line number Diff line number Diff line
@@ -37,31 +37,6 @@ const Select = function (source, criteria = {}, options = {}) { 
  this.forUpdate = options.forUpdate;

  this.forShare = options.forShare;



  switch (options.order) {

    case null:

      // null options.order indicates ORDER BY should be omitted entirely for

      // aggregation etc

      this.order = '';

      break;

    case undefined:

      if (source.loader === 'join' && source.pk) {

        const delimitedPks = source.pk.map(pk => `${source.delimitedFullName}."${pk}"`);



        this.order = `ORDER BY ${delimitedPks.join(', ')}`;

      } else if (source.pk) {

        const delimitedPks = source.pk.map(pk => `"${pk}"`);



        this.order = `ORDER BY ${delimitedPks.join(', ')}`;

      } else {

        this.order = 'ORDER BY 1';

      }



      break;

    default:

      this.order = orderBy(options.order, source, options.orderBody);

      break;

  }



  this.fields = [];



  // add user-defined fields
@@ -119,11 +94,15 @@ const Select = function (source, criteria = {}, options = {}) { 
  this.where = where(source, criteria, 0, this.generator);

  this.params = this.where.params;



  if (options.order) {

    this.order = orderBy(options.order, source, options.orderBody);

  }



  // with pageLength set for keyset pagination, add last values of ordering

  // fields to criteria

  if (this.pageLength) {

    if (!Object.prototype.hasOwnProperty.call(options, 'order')) {

      this.error = 'Keyset paging with pageLength requires options.order';

    if (!this.order) {

      this.error = 'Keyset paging with pageLength requires an explicit order directive';

    } else if (

      Object.prototype.hasOwnProperty.call(options, 'offset') ||

      Object.prototype.hasOwnProperty.call(options, 'limit')
@@ -172,14 +151,11 @@ Select.prototype.format = function () { 
  sql += `WHERE ${this.where.conditions}`;



  if (this.pagination) { sql += ` AND ${this.pagination}`; }



  // TODO only if explicit?

  sql += `${this.order}`;



  if (this.order) { sql += ` ${this.order}`; }

  if (this.forUpdate) { sql += ' FOR UPDATE'; }

  if (this.forShare) { sql += ' FOR SHARE'; }

  if (this.pageLength) { sql += ` FETCH FIRST ${this.pageLength} ROWS ONLY`; }

  if (this.offset) { sql += ' OFFSET ' + this.offset; }

  if (this.offset) { sql += ` OFFSET ${this.offset}`; }

  if (this.single) {

    sql += ' LIMIT 1';

  } else if (this.limit) {

test/database/attach.js

+1 −1
Changes for test/database/attach.js: 1 added line, 1 removed line.
Original line number Diff line number Diff line
@@ -248,7 +248,7 @@ describe('attaching entities', function () { 


      assert.equal(result1[0].val, 1);

      assert.deepEqual(result2, {

        sql: 'SELECT * FROM "function_or_table" WHERE TRUE ORDER BY 1',

        sql: 'SELECT * FROM "function_or_table" WHERE TRUE',

        params: []

      });

    });

test/database/query.js

+1 −1
Changes for test/database/query.js: 1 added line, 1 removed line.
Original line number Diff line number Diff line
@@ -115,7 +115,7 @@ describe('query', function () { 
    return db.query(query).then(result => {

      assert.isObject(result);

      assert.deepEqual(result, {

        sql: `SELECT "id" FROM "products" WHERE "id" = $1 ORDER BY "id"`,

        sql: `SELECT "id" FROM "products" WHERE "id" = $1`,

        params: [1]

      });

    });

test/readable/decomposition.js

+1 −0
Changes for test/readable/decomposition.js: 1 added line, 0 removed lines.
Original line number Diff line number Diff line
@@ -25,6 +25,7 @@ describe('decomposing results', function () { 


  it('applies a schema to decompose results', function* () {

    const issues = yield db.everything.find({}, {

      order: [{field: 'user_id'}],

      decompose: {

        pk: 'user_id',

        columns: {

test/readable/find.js

+10 −4
Changes for test/readable/find.js: 10 added lines, 4 removed lines.
Original line number Diff line number Diff line
@@ -512,24 +512,30 @@ describe('find', function () { 
    it('returns all records on find with no args', function () {

      return db.popular_products.find().then(res => assert.lengthOf(res, 3));

    });

    it('returns first record with findOne no args', function () {

      return db.popular_products.findOne().then(res => assert.equal(res.id, 1));



    it('returns one record with findOne no args', function () {

      return db.popular_products.findOne().then(res => assert.isAbove(res.id, 0));

    });



    it('handles multiple predicates', function () {

      return db.popular_products.where('price=$1 OR price=$2', [12.00, 24.00]).then(res => assert.lengthOf(res, 2));

    });



    it('counts rows with where-style args', function () {

      return db.popular_products.count('price=$1 OR price=$2', [12.00, 24.00]).then(res => assert.equal(res, 2));

    });



    it('counts rows with find-style args', function () {

      return db.popular_products.count({price: [12.00, 24.00]}).then(res => assert.equal(res, 2));

    });



    it('makes comparisons', function () {

      return db.popular_products.find({'price > ': 30.00}).then(res => {

        assert.lengthOf(res, 1);

        assert.equal(res[0].id, 4);

      });

    });



    it('rejects if you try to search by pk', function () {

      return db.popular_products.find(1).then(() => { assert.fail(); }).catch(() => {});

    });
@@ -552,8 +558,8 @@ describe('find', function () { 


    it('allows expressions in the select list', function () {

      return db.popular_products.find({}, {fields: ['id'], exprs: {name: 'upper(name)'}}).then(res => {

        assert.equal(res[0].id, 1);

        assert.equal(res[0].name, 'PRODUCT 1');

        assert.isAbove(res[0].id, 0);

        assert.equal(res[0].name, `PRODUCT ${res[0].id}`);

      });

    });