Display null values as last while queries with ORDER_BY

Add NULLS LAST option to sql queries with ORDER_BY closes LiskArchive#356
MaciejBaj · Jan 5, 2017 · 1eae01e · 1eae01e
1 parent 01e6541
commit 1eae01e
Show file tree

Hide file tree

Showing 9 changed files with 39 additions and 13 deletions.
diff --git a/sql/blocks.js b/sql/blocks.js
@@ -36,14 +36,14 @@ var BlocksSql = {
     return [
       'SELECT * FROM blocks_list',
       (params.where.length ? 'WHERE ' + params.where.join(' AND ') : ''),
-      (params.sortField ? 'ORDER BY ' + [params.sortField, params.sortMethod].join(' ') : ''),
+      (params.sortField ? 'ORDER BY ' + [params.sortField, params.sortMethod].join(' ') + ' NULLS LAST' : ''),
       'LIMIT ${limit} OFFSET ${offset}'
     ].filter(Boolean).join(' ');
   },
 
   getById: 'SELECT * FROM blocks_list WHERE "b_id" = ${id}',
 
-  getIdSequence: 'SELECT (ARRAY_AGG("id" ORDER BY "height" ASC))[1] AS "id", MIN("height") AS "height", CAST("height" / ${delegates} AS INTEGER) + (CASE WHEN "height" % ${activeDelegates} > 0 THEN 1 ELSE 0 END) AS "round" FROM blocks WHERE "height" <= ${height} GROUP BY "round" ORDER BY "height" DESC LIMIT ${limit}',
+  getIdSequence: 'SELECT (ARRAY_AGG("id" ORDER BY "height" ASC NULLS LAST))[1] AS "id", MIN("height") AS "height", CAST("height" / ${delegates} AS INTEGER) + (CASE WHEN "height" % ${activeDelegates} > 0 THEN 1 ELSE 0 END) AS "round" FROM blocks WHERE "height" <= ${height} GROUP BY "round" ORDER BY "height" DESC NULLS LAST LIMIT ${limit}',
 
   getCommonBlock: function (params) {
     return [
@@ -75,9 +75,9 @@ var BlocksSql = {
     ].filter(Boolean).join(' ');
   },
 
-  loadBlocksOffset: 'SELECT * FROM full_blocks_list WHERE "b_height" >= ${offset} AND "b_height" < ${limit} ORDER BY "b_height", "t_rowId"',
+  loadBlocksOffset: 'SELECT * FROM full_blocks_list WHERE "b_height" >= ${offset} AND "b_height" < ${limit} ORDER BY "b_height", "t_rowId" NULLS LAST',
 
-  loadLastBlock: 'SELECT * FROM full_blocks_list WHERE "b_height" = (SELECT MAX("height") FROM blocks) ORDER BY "b_height", "t_rowId"',
+  loadLastBlock: 'SELECT * FROM full_blocks_list WHERE "b_height" = (SELECT MAX("height") FROM blocks) ORDER BY "b_height", "t_rowId" NULLS LAST',
 
   getBlockId: 'SELECT "id" FROM blocks WHERE "id" = ${id}',
 

diff --git a/sql/dapps.js b/sql/dapps.js
@@ -27,7 +27,7 @@ var DappsSql = {
     return [
       'SELECT "name", "description", "tags", "link", "type", "category", "icon", "transactionId" FROM dapps',
       (params.where.length ? 'WHERE ' + params.where.join(' OR ') : ''),
-      (params.sortField ? 'ORDER BY ' + [params.sortField, params.sortMethod].join(' ') : ''),
+      (params.sortField ? 'ORDER BY ' + [params.sortField, params.sortMethod].join(' ') + ' NULLS LAST' : ''),
       'LIMIT ${limit} OFFSET ${offset}'
     ].filter(Boolean).join(' ');
   },
@@ -36,15 +36,15 @@ var DappsSql = {
 
   getCommonBlock: 'SELECT b."height" AS "height", t."id" AS "id", t."senderId" AS "senderId", t."amount" AS "amount" FROM trs t INNER JOIN blocks b ON t."blockId" = b."id" AND t."id" = ${id} AND t."type" = ${type} INNER JOIN intransfer dt ON dt."transactionId" = t."id" AND dt."dappid" = ${dappid}',
 
-  getWithdrawalLastTransaction: 'SELECT ot."outTransactionId" FROM trs t INNER JOIN blocks b ON t."blockId" = b."id" AND t."type" = ${type} INNER JOIN outtransfer ot ON ot."transactionId" = t."id" AND ot."dappId" = ${dappid} ORDER BY b."height" DESC LIMIT 1',
+  getWithdrawalLastTransaction: 'SELECT ot."outTransactionId" FROM trs t INNER JOIN blocks b ON t."blockId" = b."id" AND t."type" = ${type} INNER JOIN outtransfer ot ON ot."transactionId" = t."id" AND ot."dappId" = ${dappid} ORDER BY b."height" DESC LIMIT 1 NULLS LAST',
 
   getBalanceTransactions: function (params) {
     return [
       'SELECT t."id" AS "id", ENCODE(t."senderPublicKey", \'hex\') AS "senderPublicKey", t."amount" AS "amount" FROM trs t',
       'INNER JOIN blocks b ON t."blockId" = b."id" AND t."type" = ${type}',
       'INNER JOIN intransfer dt ON dt."transactionId" = t."id" AND dt."dappId" = ${dappid}',
       (params.lastId ? 'WHERE b."height" > (SELECT "height" FROM blocks ib INNER JOIN trs it ON ib."id" = it."blockId" AND it."id" = ${lastId})' : ''),
-      'ORDER BY b."height"'
+      'ORDER BY b."height" NULLS LAST'
     ].filter(Boolean).join(' ');
   }
 };

diff --git a/sql/delegates.js b/sql/delegates.js
@@ -19,7 +19,7 @@ var DelegatesSql = {
       'SELECT m."username", m."address", ENCODE(m."publicKey", \'hex\') AS "publicKey", m."vote", m."producedblocks", m."missedblocks"',
       'FROM mem_accounts m',
       'WHERE m."isDelegate" = 1 AND m."username" LIKE ${q}',
-      'ORDER BY ' + [params.sortField, params.sortMethod].join(' '),
+      'ORDER BY ' + [params.sortField, params.sortMethod].join(' ') + ' NULLS LAST',
       'LIMIT ${limit}'
     ].join(' ');
 

diff --git a/sql/loader.js b/sql/loader.js
@@ -5,7 +5,7 @@ var LoaderSql = {
 
   getGenesisBlock: 'SELECT "id", "payloadHash", "blockSignature" FROM blocks WHERE "height" = 1',
 
-  countMemAccounts: 'SELECT COUNT(*)::int FROM mem_accounts WHERE "blockId" = (SELECT "id" FROM "blocks" ORDER BY "height" DESC LIMIT 1)',
+  countMemAccounts: 'SELECT COUNT(*)::int FROM mem_accounts WHERE "blockId" = (SELECT "id" FROM "blocks" ORDER BY "height" DESC NULLS LAST LIMIT 1)',
 
   getMemRounds: 'SELECT "round" FROM mem_round GROUP BY "round"',
 

diff --git a/sql/peers.js b/sql/peers.js
@@ -18,7 +18,7 @@ var PeersSql = {
     return [
       'SELECT "ip", "port", "state", "os", "version", ENCODE("broadhash", \'hex\') AS "broadhash", "height" FROM peers',
       (params.where.length ? 'WHERE ' + params.where.join(' AND ') : ''),
-      (params.sortField ? 'ORDER BY ' + [params.sortField, params.sortMethod].join(' ') : 'ORDER BY RANDOM()'),
+      (params.sortField ? 'ORDER BY ' + [params.sortField, params.sortMethod].join(' ') + ' NULLS LAST' : 'ORDER BY RANDOM()'),
       'LIMIT ${limit} OFFSET ${offset}'
     ].filter(Boolean).join(' ');
   },

diff --git a/sql/system.js b/sql/system.js
@@ -1,7 +1,7 @@
 'use strict';
 
 var SystemSql = {
-  getBroadhash: 'SELECT "id" FROM blocks ORDER BY "height" DESC LIMIT ${limit}'
+  getBroadhash: 'SELECT "id" FROM blocks ORDER BY "height" DESC NULLS LAST LIMIT ${limit}'
 };
 
 module.exports = SystemSql;
diff --git a/sql/transactions.js b/sql/transactions.js
@@ -36,7 +36,7 @@ var TransactionsSql = {
       (params.where.length || params.owner ? 'WHERE' : ''),
       (params.where.length ? '(' + params.where.join(' OR ') + ')' : ''),
       (params.where.length && params.owner ? ' AND ' + params.owner : params.owner),
-      (params.sortField ? 'ORDER BY ' + [params.sortField, params.sortMethod].join(' ') : ''),
+      (params.sortField ? 'ORDER BY ' + [params.sortField, params.sortMethod].join(' ') + ' NULLS LAST' : ''),
       'LIMIT ${limit} OFFSET ${offset}'
     ].filter(Boolean).join(' ');
   },

diff --git a/sql/transport.js b/sql/transport.js
@@ -1,7 +1,7 @@
 'use strict';
 
 var TransportSql = {
-  getCommonBlock: 'SELECT MAX("height") AS "height", "id", "previousBlock", "timestamp" FROM blocks WHERE "id" IN ($1:csv) GROUP BY "id" ORDER BY "height" DESC'
+  getCommonBlock: 'SELECT MAX("height") AS "height", "id", "previousBlock", "timestamp" FROM blocks WHERE "id" IN ($1:csv) GROUP BY "id" ORDER BY "height" DESC NULLS LAST'
 };
 
 module.exports = TransportSql;
diff --git a/test/api/peers.js b/test/api/peers.js
@@ -348,6 +348,32 @@ describe('GET /api/peers', function () {
 		});
 	});
 
+    it('using orderBy == "height:desc" should not place NULLs first', function (done) {
+        var orderBy = 'height:desc';
+        var params = 'orderBy=' + orderBy;
+
+        node.get('/api/peers?' + params, function (err, res) {
+            node.expect(res.body).to.have.property('success').to.be.ok;
+            node.expect(res.body).to.have.property('peers').that.is.an('array');
+
+            var dividedIndices = res.body.peers.reduce(function (memo, peer, index) {
+            	memo[peer.height === null ? 'nullIndices' : 'notNullIndices'].push(index);
+            	return memo;
+            }, {notNullIndices: [], nullIndices: []});
+
+            if (dividedIndices.nullIndices.length) {
+                var ascOrder = function (a, b) { return a - b };
+                dividedIndices.notNullIndices.sort(ascOrder);
+                dividedIndices.nullIndices.sort(ascOrder);
+
+                node.expect(dividedIndices.notNullIndices[dividedIndices.notNullIndices.length - 1])
+                    .to.be.at.most(dividedIndices.nullIndices[0]);
+			}
+
+            done();
+        });
+    });
+
 	it('using string limit should fail', function (done) {
 		var limit = 'one';
 		var params = 'limit=' + limit;