From dcb9b8ec5213b570d53b789fedc60109c247900e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 14 Sep 2017 17:56:17 +0200 Subject: [PATCH 01/21] Rename BaseWidget by BaseDataview --- lib/cartodb/models/dataview/formula.js | 51 +++++++++++++++----------- 1 file changed, 30 insertions(+), 21 deletions(-) diff --git a/lib/cartodb/models/dataview/formula.js b/lib/cartodb/models/dataview/formula.js index 7ec356b7..6781fba3 100644 --- a/lib/cartodb/models/dataview/formula.js +++ b/lib/cartodb/models/dataview/formula.js @@ -1,26 +1,35 @@ var _ = require('underscore'); -var BaseWidget = require('./base'); +var BaseDataview = require('./base'); var debug = require('debug')('windshaft:widget:formula'); -var dot = require('dot'); -dot.templateSettings.strip = false; +const countInfinitiesQueryTpl = ctx => ` + SELECT count(1) FROM (${ctx._query}) __cdb_formula_infinities + WHERE ${ctx._column} = 'infinity'::float OR ${ctx._column} = '-infinity'::float +`; -var formulaQueryTpl = dot.template([ - 'SELECT', - ' {{=it._operation}}({{=it._column}}) AS result,', - ' (SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nulls WHERE {{=it._column}} IS NULL) AS nulls_count', - ' {{?it._isFloatColumn}},(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nulls', - ' WHERE {{=it._column}} = \'infinity\'::float OR {{=it._column}} = \'-infinity\'::float) AS infinities_count', - ' ,(SELECT count(1) FROM ({{=it._query}}) _cdb_formula_nulls', - ' WHERE {{=it._column}} = \'NaN\'::float) AS nans_count{{?}}', - 'FROM ({{=it._query}}) _cdb_formula', - '{{?it._isFloatColumn && it._operation !== \'count\'}}WHERE', - ' {{=it._column}} != \'infinity\'::float', - 'AND', - ' {{=it._column}} != \'-infinity\'::float', - 'AND', - ' {{=it._column}} != \'NaN\'::float{{?}}' -].join('\n')); +const countNansQueryTpl = ctx => ` + SELECT count(1) FROM (${ctx._query}) __cdb_formula_nans + WHERE ${ctx._column} = 'NaN'::float +`; + +const filterOutSpecialNumericValuesTpl = ctx => ` + WHERE + ${ctx._column} != 'infinity'::float + AND + ${ctx._column} != '-infinity'::float + AND + ${ctx._column} != 'NaN'::float +`; + +const formulaQueryTpl = ctx => ` + SELECT + ${ctx._operation}(${ctx._column}) AS result, + (SELECT count(1) FROM (${ctx._query}) _cdb_formula_nulls WHERE ${ctx._column} IS NULL) AS nulls_count + ${ctx._isFloatColumn ? `,(${countInfinitiesQueryTpl(ctx)}) AS infinities_count` : ''} + ${ctx._isFloatColumn ? `,(${countNansQueryTpl(ctx)}) AS nans_count` : ''} + FROM (${ctx._query}) __cdb_formula + ${ctx._isFloatColumn && ctx._operation !== 'count' ? `${filterOutSpecialNumericValuesTpl(ctx)}` : ''} +`; var VALID_OPERATIONS = { count: true, @@ -54,7 +63,7 @@ function Formula(query, options, queries) { throw new Error('Formula expects `column` in widget options'); } - BaseWidget.apply(this); + BaseDataview.apply(this); this.query = query; this.queries = queries; @@ -63,7 +72,7 @@ function Formula(query, options, queries) { this._isFloatColumn = null; } -Formula.prototype = new BaseWidget(); +Formula.prototype = new BaseDataview(); Formula.prototype.constructor = Formula; module.exports = Formula; From 1063d81c1bb64a71c1ccc14f0626005b7887f3cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 14 Sep 2017 17:56:40 +0200 Subject: [PATCH 02/21] Rename debug namespace --- lib/cartodb/models/dataview/formula.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cartodb/models/dataview/formula.js b/lib/cartodb/models/dataview/formula.js index 6781fba3..54bbe430 100644 --- a/lib/cartodb/models/dataview/formula.js +++ b/lib/cartodb/models/dataview/formula.js @@ -1,6 +1,6 @@ var _ = require('underscore'); var BaseDataview = require('./base'); -var debug = require('debug')('windshaft:widget:formula'); +var debug = require('debug')('windshaft:dataview:formula'); const countInfinitiesQueryTpl = ctx => ` SELECT count(1) FROM (${ctx._query}) __cdb_formula_infinities From a4ecc18f2fb98db46b3b8ab400f1fc5cee4836d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 14 Sep 2017 17:57:24 +0200 Subject: [PATCH 03/21] Use default values for constructor's arguments --- lib/cartodb/models/dataview/formula.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cartodb/models/dataview/formula.js b/lib/cartodb/models/dataview/formula.js index 54bbe430..4c7f185b 100644 --- a/lib/cartodb/models/dataview/formula.js +++ b/lib/cartodb/models/dataview/formula.js @@ -50,7 +50,7 @@ var TYPE = 'formula'; } } */ -function Formula(query, options, queries) { +function Formula(query, options = {}, queries = {}) { if (!_.isString(options.operation)) { throw new Error('Formula expects `operation` in widget options'); } From 9c64d674b3522f80c812f7676fb1c1aa4c3c5dbf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Thu, 14 Sep 2017 18:02:13 +0200 Subject: [PATCH 04/21] Do not use underscore --- lib/cartodb/models/dataview/formula.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/cartodb/models/dataview/formula.js b/lib/cartodb/models/dataview/formula.js index 4c7f185b..36b5369b 100644 --- a/lib/cartodb/models/dataview/formula.js +++ b/lib/cartodb/models/dataview/formula.js @@ -1,4 +1,3 @@ -var _ = require('underscore'); var BaseDataview = require('./base'); var debug = require('debug')('windshaft:dataview:formula'); @@ -51,7 +50,7 @@ var TYPE = 'formula'; } */ function Formula(query, options = {}, queries = {}) { - if (!_.isString(options.operation)) { + if (typeof options.operation !== 'string') { throw new Error('Formula expects `operation` in widget options'); } @@ -59,7 +58,7 @@ function Formula(query, options = {}, queries = {}) { throw new Error("Formula does not support '" + options.operation + "' operation"); } - if (options.operation !== 'count' && !_.isString(options.column)) { + if (options.operation !== 'count' && typeof options.column !== 'string') { throw new Error('Formula expects `column` in widget options'); } From 2437288d9d1675f226a6c8e0aa6f7c4dc233d7ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 15 Sep 2017 10:37:51 +0200 Subject: [PATCH 05/21] Replace widget word by dataview --- lib/cartodb/models/dataview/formula.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/cartodb/models/dataview/formula.js b/lib/cartodb/models/dataview/formula.js index 36b5369b..b96e05b1 100644 --- a/lib/cartodb/models/dataview/formula.js +++ b/lib/cartodb/models/dataview/formula.js @@ -51,15 +51,15 @@ var TYPE = 'formula'; */ function Formula(query, options = {}, queries = {}) { if (typeof options.operation !== 'string') { - throw new Error('Formula expects `operation` in widget options'); + throw new Error(`Formula expects 'operation' in dataview options`); } if (!VALID_OPERATIONS[options.operation]) { - throw new Error("Formula does not support '" + options.operation + "' operation"); + throw new Error(`Formula does not support '${options.operation}' operation`) } if (options.operation !== 'count' && typeof options.column !== 'string') { - throw new Error('Formula expects `column` in widget options'); + throw new Error(`Formula expects 'column' in dataview options`); } BaseDataview.apply(this); From 68dfed8b85df674685c7e3569f2d2697e2f88bae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 15 Sep 2017 10:48:44 +0200 Subject: [PATCH 06/21] Use ES6 class syntax --- lib/cartodb/models/dataview/formula.js | 146 ++++++++++++------------- 1 file changed, 71 insertions(+), 75 deletions(-) diff --git a/lib/cartodb/models/dataview/formula.js b/lib/cartodb/models/dataview/formula.js index b96e05b1..ba6725e3 100644 --- a/lib/cartodb/models/dataview/formula.js +++ b/lib/cartodb/models/dataview/formula.js @@ -49,93 +49,89 @@ var TYPE = 'formula'; } } */ -function Formula(query, options = {}, queries = {}) { - if (typeof options.operation !== 'string') { - throw new Error(`Formula expects 'operation' in dataview options`); +module.exports = class Formula extends BaseDataview { + constructor (query, options = {}, queries = {}) { + if (typeof options.operation !== 'string') { + throw new Error(`Formula expects 'operation' in dataview options`); + } + + if (!VALID_OPERATIONS[options.operation]) { + throw new Error(`Formula does not support '${options.operation}' operation`) + } + + if (options.operation !== 'count' && typeof options.column !== 'string') { + throw new Error(`Formula expects 'column' in dataview options`); + } + + this.query = query; + this.queries = queries; + this.column = options.column || '1'; + this.operation = options.operation; + this._isFloatColumn = null; } - if (!VALID_OPERATIONS[options.operation]) { - throw new Error(`Formula does not support '${options.operation}' operation`) - } - if (options.operation !== 'count' && typeof options.column !== 'string') { - throw new Error(`Formula expects 'column' in dataview options`); - } + sql (psql, override, callback) { + var self = this; - BaseDataview.apply(this); + if (!callback) { + callback = override; + override = {}; + } - this.query = query; - this.queries = queries; - this.column = options.column || '1'; - this.operation = options.operation; - this._isFloatColumn = null; -} + if (this._isFloatColumn === null) { + this._isFloatColumn = false; + this.getColumnType(psql, this.column, this.queries.no_filters, function (err, type) { + if (!err && !!type) { + self._isFloatColumn = type.float; + } + self.sql(psql, override, callback); + }); + return null; + } -Formula.prototype = new BaseDataview(); -Formula.prototype.constructor = Formula; - -module.exports = Formula; - -Formula.prototype.sql = function(psql, override, callback) { - var self = this; - - if (!callback) { - callback = override; - override = {}; - } - - if (this._isFloatColumn === null) { - this._isFloatColumn = false; - this.getColumnType(psql, this.column, this.queries.no_filters, function (err, type) { - if (!err && !!type) { - self._isFloatColumn = type.float; - } - self.sql(psql, override, callback); + var formulaSql = formulaQueryTpl({ + _isFloatColumn: this._isFloatColumn, + _query: this.query, + _operation: this.operation, + _column: this.column }); - return null; + + debug(formulaSql); + + return callback(null, formulaSql); } - var formulaSql = formulaQueryTpl({ - _isFloatColumn: this._isFloatColumn, - _query: this.query, - _operation: this.operation, - _column: this.column - }); + format (result) { + var formattedResult = { + operation: this.operation, + result: 0, + nulls: 0, + nans: 0, + infinities: 0 + }; - debug(formulaSql); + if (result.rows.length) { + formattedResult.operation = this.operation; + formattedResult.result = result.rows[0].result; + formattedResult.nulls = result.rows[0].nulls_count; + formattedResult.nans = result.rows[0].nans_count; + formattedResult.infinities = result.rows[0].infinities_count; + } - return callback(null, formulaSql); -}; + return formattedResult; + } -Formula.prototype.format = function(result) { - var formattedResult = { - operation: this.operation, - result: 0, - nulls: 0, - nans: 0, - infinities: 0 + getType () { + return TYPE; }; - if (result.rows.length) { - formattedResult.operation = this.operation; - formattedResult.result = result.rows[0].result; - formattedResult.nulls = result.rows[0].nulls_count; - formattedResult.nans = result.rows[0].nans_count; - formattedResult.infinities = result.rows[0].infinities_count; + toString () { + return JSON.stringify({ + _type: TYPE, + _query: this.query, + _column: this.column, + _operation: this.operation + }); } - - return formattedResult; -}; - -Formula.prototype.getType = function() { - return TYPE; -}; - -Formula.prototype.toString = function() { - return JSON.stringify({ - _type: TYPE, - _query: this.query, - _column: this.column, - _operation: this.operation - }); -}; +} From ecbc7a28e769bb273556b56d8730121b5c8c185b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 15 Sep 2017 10:49:20 +0200 Subject: [PATCH 07/21] Declare constants with const keyword --- lib/cartodb/models/dataview/formula.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/cartodb/models/dataview/formula.js b/lib/cartodb/models/dataview/formula.js index ba6725e3..efaeb0c6 100644 --- a/lib/cartodb/models/dataview/formula.js +++ b/lib/cartodb/models/dataview/formula.js @@ -30,7 +30,7 @@ const formulaQueryTpl = ctx => ` ${ctx._isFloatColumn && ctx._operation !== 'count' ? `${filterOutSpecialNumericValuesTpl(ctx)}` : ''} `; -var VALID_OPERATIONS = { +const VALID_OPERATIONS = { count: true, avg: true, sum: true, @@ -38,7 +38,7 @@ var VALID_OPERATIONS = { max: true }; -var TYPE = 'formula'; +const TYPE = 'formula'; /** { From c00a93f4144abf7eccaf9c7691c2138218866950 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 15 Sep 2017 10:58:11 +0200 Subject: [PATCH 08/21] Use destruturing assignment to format the formula result --- lib/cartodb/models/dataview/formula.js | 27 ++++++++++++-------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/lib/cartodb/models/dataview/formula.js b/lib/cartodb/models/dataview/formula.js index efaeb0c6..2172beab 100644 --- a/lib/cartodb/models/dataview/formula.js +++ b/lib/cartodb/models/dataview/formula.js @@ -103,23 +103,20 @@ module.exports = class Formula extends BaseDataview { } format (result) { - var formattedResult = { + const { + result = 0, + nulls_count = 0, + nans_count = 0, + infinities_count = 0 + } = result.rows[0] || {}; + + return { operation: this.operation, - result: 0, - nulls: 0, - nans: 0, - infinities: 0 + result, + nulls: nulls_count, + nans: nans_count, + infinities: infinities_count }; - - if (result.rows.length) { - formattedResult.operation = this.operation; - formattedResult.result = result.rows[0].result; - formattedResult.nulls = result.rows[0].nulls_count; - formattedResult.nans = result.rows[0].nans_count; - formattedResult.infinities = result.rows[0].infinities_count; - } - - return formattedResult; } getType () { From 9771979b8fc391dc396f3c45be1effb6659503d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 15 Sep 2017 10:59:07 +0200 Subject: [PATCH 09/21] Missing call to super class in constructor --- lib/cartodb/models/dataview/formula.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/cartodb/models/dataview/formula.js b/lib/cartodb/models/dataview/formula.js index 2172beab..c2528a91 100644 --- a/lib/cartodb/models/dataview/formula.js +++ b/lib/cartodb/models/dataview/formula.js @@ -51,6 +51,8 @@ const TYPE = 'formula'; */ module.exports = class Formula extends BaseDataview { constructor (query, options = {}, queries = {}) { + super(); + if (typeof options.operation !== 'string') { throw new Error(`Formula expects 'operation' in dataview options`); } From 11f7b38c6975144ff3ac9abfb0aae19f085e23c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 15 Sep 2017 11:54:56 +0200 Subject: [PATCH 10/21] Do not use dot module to build column type query --- lib/cartodb/models/dataview/base.js | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/lib/cartodb/models/dataview/base.js b/lib/cartodb/models/dataview/base.js index 29069d37..fb5f520e 100644 --- a/lib/cartodb/models/dataview/base.js +++ b/lib/cartodb/models/dataview/base.js @@ -1,6 +1,3 @@ -var dot = require('dot'); -dot.templateSettings.strip = false; - function BaseDataview() {} module.exports = BaseDataview; @@ -43,15 +40,14 @@ var DATE_OIDS = { 1184: true }; -var columnTypeQueryTpl = dot.template( - 'SELECT pg_typeof({{=it.column}})::oid FROM ({{=it.query}}) _cdb_column_type limit 1' -); +var columnTypeQueryTpl = ctx => `SELECT pg_typeof(${ctx.column})::oid FROM (${ctx.query}) _cdb_column_type limit 1`; BaseDataview.prototype.getColumnType = function (psql, column, query, callback) { var readOnlyTransaction = true; var columnTypeQuery = columnTypeQueryTpl({ - column: column, query: query + column: column, + query: query }); psql.query(columnTypeQuery, function(err, result) { From b4ce13e4293c5770d4852415c2e1e33f1f7d4219 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 15 Sep 2017 11:56:59 +0200 Subject: [PATCH 11/21] Use object shorthand notation --- lib/cartodb/models/dataview/base.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/cartodb/models/dataview/base.js b/lib/cartodb/models/dataview/base.js index fb5f520e..cacfaa77 100644 --- a/lib/cartodb/models/dataview/base.js +++ b/lib/cartodb/models/dataview/base.js @@ -45,10 +45,7 @@ var columnTypeQueryTpl = ctx => `SELECT pg_typeof(${ctx.column})::oid FROM (${ct BaseDataview.prototype.getColumnType = function (psql, column, query, callback) { var readOnlyTransaction = true; - var columnTypeQuery = columnTypeQueryTpl({ - column: column, - query: query - }); + var columnTypeQuery = columnTypeQueryTpl({ column, query }); psql.query(columnTypeQuery, function(err, result) { if (err) { From ada58f6ea2a5c8590193ab67359655f78af69f52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 15 Sep 2017 13:35:00 +0200 Subject: [PATCH 12/21] Use const keyword to declare varibles --- lib/cartodb/models/dataview/base.js | 91 ++++++++++++++--------------- 1 file changed, 44 insertions(+), 47 deletions(-) diff --git a/lib/cartodb/models/dataview/base.js b/lib/cartodb/models/dataview/base.js index cacfaa77..bc7d13bd 100644 --- a/lib/cartodb/models/dataview/base.js +++ b/lib/cartodb/models/dataview/base.js @@ -1,60 +1,16 @@ -function BaseDataview() {} - -module.exports = BaseDataview; - -BaseDataview.prototype.getResult = function(psql, override, callback) { - var self = this; - this.sql(psql, override, function(err, query) { - if (err) { - return callback(err); - } - - psql.query(query, function(err, result) { - if (err) { - return callback(err, result); - } - - result = self.format(result, override); - result.type = self.getType(); - - return callback(null, result); - - }, true); // use read-only transaction - }); - -}; - -BaseDataview.prototype.search = function(psql, userQuery, callback) { - return callback(null, this.format({ rows: [] })); -}; - -var FLOAT_OIDS = { +const FLOAT_OIDS = { 700: true, 701: true, 1700: true }; -var DATE_OIDS = { +const DATE_OIDS = { 1082: true, 1114: true, 1184: true }; -var columnTypeQueryTpl = ctx => `SELECT pg_typeof(${ctx.column})::oid FROM (${ctx.query}) _cdb_column_type limit 1`; - -BaseDataview.prototype.getColumnType = function (psql, column, query, callback) { - var readOnlyTransaction = true; - - var columnTypeQuery = columnTypeQueryTpl({ column, query }); - - psql.query(columnTypeQuery, function(err, result) { - if (err) { - return callback(err); - } - var pgType = result.rows[0].pg_typeof; - callback(null, getPGTypeName(pgType)); - }, readOnlyTransaction); -}; +const columnTypeQueryTpl = ctx => `SELECT pg_typeof(${ctx.column})::oid FROM (${ctx.query}) _cdb_column_type limit 1`; function getPGTypeName (pgType) { return { @@ -62,3 +18,44 @@ function getPGTypeName (pgType) { date: DATE_OIDS.hasOwnProperty(pgType) }; } + +module.exports = class BaseDataview { + getResult (psql, override, callback) { + const self = this; + this.sql(psql, override, function(err, query) { + if (err) { + return callback(err); + } + + psql.query(query, function(err, result) { + if (err) { + return callback(err, result); + } + + result = self.format(result, override); + result.type = self.getType(); + + return callback(null, result); + + }, true); // use read-only transaction + }); + } + + search (psql, userQuery, callback) { + return callback(null, this.format({ rows: [] })); + }; + + getColumnType (psql, column, query, callback) { + const readOnlyTransaction = true; + + const columnTypeQuery = columnTypeQueryTpl({ column, query }); + + psql.query(columnTypeQuery, function(err, result) { + if (err) { + return callback(err); + } + const pgType = result.rows[0].pg_typeof; + callback(null, getPGTypeName(pgType)); + }, readOnlyTransaction); + } +}; From 90c4796d4ede54f649f0f8d7112afae325c32b7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 15 Sep 2017 13:41:54 +0200 Subject: [PATCH 13/21] Remove empty line --- lib/cartodb/models/dataview/base.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/cartodb/models/dataview/base.js b/lib/cartodb/models/dataview/base.js index bc7d13bd..702f43e5 100644 --- a/lib/cartodb/models/dataview/base.js +++ b/lib/cartodb/models/dataview/base.js @@ -43,11 +43,10 @@ module.exports = class BaseDataview { search (psql, userQuery, callback) { return callback(null, this.format({ rows: [] })); - }; + } getColumnType (psql, column, query, callback) { const readOnlyTransaction = true; - const columnTypeQuery = columnTypeQueryTpl({ column, query }); psql.query(columnTypeQuery, function(err, result) { From 1959a841fd8823383f5e37e3bf7028a2f71377b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 15 Sep 2017 14:26:22 +0200 Subject: [PATCH 14/21] Use arrow function to take advantage of bound context --- lib/cartodb/models/dataview/base.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/cartodb/models/dataview/base.js b/lib/cartodb/models/dataview/base.js index 702f43e5..ea7f9474 100644 --- a/lib/cartodb/models/dataview/base.js +++ b/lib/cartodb/models/dataview/base.js @@ -21,8 +21,7 @@ function getPGTypeName (pgType) { module.exports = class BaseDataview { getResult (psql, override, callback) { - const self = this; - this.sql(psql, override, function(err, query) { + this.sql(psql, override, (err, query) => { if (err) { return callback(err); } @@ -32,8 +31,8 @@ module.exports = class BaseDataview { return callback(err, result); } - result = self.format(result, override); - result.type = self.getType(); + result = this.format(result, override); + result.type = this.getType(); return callback(null, result); From c7ed3d34e899ba6b76ee96755d85b140ab0c5725 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 15 Sep 2017 11:35:06 +0200 Subject: [PATCH 15/21] Use const instead of var to declare variables --- lib/cartodb/models/dataview/formula.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/cartodb/models/dataview/formula.js b/lib/cartodb/models/dataview/formula.js index c2528a91..1ce0050b 100644 --- a/lib/cartodb/models/dataview/formula.js +++ b/lib/cartodb/models/dataview/formula.js @@ -1,5 +1,5 @@ -var BaseDataview = require('./base'); -var debug = require('debug')('windshaft:dataview:formula'); +const BaseDataview = require('./base'); +const debug = require('debug')('windshaft:dataview:formula'); const countInfinitiesQueryTpl = ctx => ` SELECT count(1) FROM (${ctx._query}) __cdb_formula_infinities @@ -58,7 +58,7 @@ module.exports = class Formula extends BaseDataview { } if (!VALID_OPERATIONS[options.operation]) { - throw new Error(`Formula does not support '${options.operation}' operation`) + throw new Error(`Formula does not support '${options.operation}' operation`); } if (options.operation !== 'count' && typeof options.column !== 'string') { @@ -74,7 +74,7 @@ module.exports = class Formula extends BaseDataview { sql (psql, override, callback) { - var self = this; + const self = this; if (!callback) { callback = override; @@ -92,7 +92,7 @@ module.exports = class Formula extends BaseDataview { return null; } - var formulaSql = formulaQueryTpl({ + const formulaSql = formulaQueryTpl({ _isFloatColumn: this._isFloatColumn, _query: this.query, _operation: this.operation, @@ -123,7 +123,7 @@ module.exports = class Formula extends BaseDataview { getType () { return TYPE; - }; + } toString () { return JSON.stringify({ @@ -133,4 +133,4 @@ module.exports = class Formula extends BaseDataview { _operation: this.operation }); } -} +}; From 419b29e60900cf5e6100835822899f7cc5fa3d0e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 15 Sep 2017 11:39:06 +0200 Subject: [PATCH 16/21] Do not prefix with '_' template context --- lib/cartodb/models/dataview/formula.js | 34 +++++++++++++------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/lib/cartodb/models/dataview/formula.js b/lib/cartodb/models/dataview/formula.js index 1ce0050b..b6faeb55 100644 --- a/lib/cartodb/models/dataview/formula.js +++ b/lib/cartodb/models/dataview/formula.js @@ -2,32 +2,32 @@ const BaseDataview = require('./base'); const debug = require('debug')('windshaft:dataview:formula'); const countInfinitiesQueryTpl = ctx => ` - SELECT count(1) FROM (${ctx._query}) __cdb_formula_infinities - WHERE ${ctx._column} = 'infinity'::float OR ${ctx._column} = '-infinity'::float + SELECT count(1) FROM (${ctx.query}) __cdb_formula_infinities + WHERE ${ctx.column} = 'infinity'::float OR ${ctx.column} = '-infinity'::float `; const countNansQueryTpl = ctx => ` - SELECT count(1) FROM (${ctx._query}) __cdb_formula_nans - WHERE ${ctx._column} = 'NaN'::float + SELECT count(1) FROM (${ctx.query}) __cdb_formula_nans + WHERE ${ctx.column} = 'NaN'::float `; const filterOutSpecialNumericValuesTpl = ctx => ` WHERE - ${ctx._column} != 'infinity'::float + ${ctx.column} != 'infinity'::float AND - ${ctx._column} != '-infinity'::float + ${ctx.column} != '-infinity'::float AND - ${ctx._column} != 'NaN'::float + ${ctx.column} != 'NaN'::float `; const formulaQueryTpl = ctx => ` SELECT - ${ctx._operation}(${ctx._column}) AS result, - (SELECT count(1) FROM (${ctx._query}) _cdb_formula_nulls WHERE ${ctx._column} IS NULL) AS nulls_count - ${ctx._isFloatColumn ? `,(${countInfinitiesQueryTpl(ctx)}) AS infinities_count` : ''} - ${ctx._isFloatColumn ? `,(${countNansQueryTpl(ctx)}) AS nans_count` : ''} - FROM (${ctx._query}) __cdb_formula - ${ctx._isFloatColumn && ctx._operation !== 'count' ? `${filterOutSpecialNumericValuesTpl(ctx)}` : ''} + ${ctx.operation}(${ctx.column}) AS result, + (SELECT count(1) FROM (${ctx.query}) _cdb_formula_nulls WHERE ${ctx.column} IS NULL) AS nulls_count + ${ctx.isFloatColumn ? `,(${countInfinitiesQueryTpl(ctx)}) AS infinities_count` : ''} + ${ctx.isFloatColumn ? `,(${countNansQueryTpl(ctx)}) AS nans_count` : ''} + FROM (${ctx.query}) __cdb_formula + ${ctx.isFloatColumn && ctx.operation !== 'count' ? `${filterOutSpecialNumericValuesTpl(ctx)}` : ''} `; const VALID_OPERATIONS = { @@ -93,10 +93,10 @@ module.exports = class Formula extends BaseDataview { } const formulaSql = formulaQueryTpl({ - _isFloatColumn: this._isFloatColumn, - _query: this.query, - _operation: this.operation, - _column: this.column + isFloatColumn: this._isFloatColumn, + query: this.query, + operation: this.operation, + column: this.column }); debug(formulaSql); From d4bb4edd1d27fba4c2ab533035c70c2762cbaf1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 15 Sep 2017 11:44:20 +0200 Subject: [PATCH 17/21] Applyy extract method to check input options --- lib/cartodb/models/dataview/formula.js | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/lib/cartodb/models/dataview/formula.js b/lib/cartodb/models/dataview/formula.js index b6faeb55..b167b87e 100644 --- a/lib/cartodb/models/dataview/formula.js +++ b/lib/cartodb/models/dataview/formula.js @@ -53,6 +53,16 @@ module.exports = class Formula extends BaseDataview { constructor (query, options = {}, queries = {}) { super(); + this._checkOptions(options); + + this.query = query; + this.queries = queries; + this.column = options.column || '1'; + this.operation = options.operation; + this._isFloatColumn = null; + } + + _checkOptions (options) { if (typeof options.operation !== 'string') { throw new Error(`Formula expects 'operation' in dataview options`); } @@ -64,12 +74,6 @@ module.exports = class Formula extends BaseDataview { if (options.operation !== 'count' && typeof options.column !== 'string') { throw new Error(`Formula expects 'column' in dataview options`); } - - this.query = query; - this.queries = queries; - this.column = options.column || '1'; - this.operation = options.operation; - this._isFloatColumn = null; } From 64c3e68303c76bc913917b6fba672cdf2a41200a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 15 Sep 2017 14:48:54 +0200 Subject: [PATCH 18/21] Fix double declaration of 'result' --- lib/cartodb/models/dataview/formula.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/cartodb/models/dataview/formula.js b/lib/cartodb/models/dataview/formula.js index b167b87e..1ad08684 100644 --- a/lib/cartodb/models/dataview/formula.js +++ b/lib/cartodb/models/dataview/formula.js @@ -108,13 +108,13 @@ module.exports = class Formula extends BaseDataview { return callback(null, formulaSql); } - format (result) { + format (res) { const { result = 0, nulls_count = 0, nans_count = 0, infinities_count = 0 - } = result.rows[0] || {}; + } = res.rows[0] || {}; return { operation: this.operation, From 61ea05d1c2b73cf0d0663bc4d4648cc83f13c5f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Fri, 15 Sep 2017 14:51:02 +0200 Subject: [PATCH 19/21] Do not assign a value by default for special float values counters --- lib/cartodb/models/dataview/formula.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/cartodb/models/dataview/formula.js b/lib/cartodb/models/dataview/formula.js index 1ad08684..c0829fa5 100644 --- a/lib/cartodb/models/dataview/formula.js +++ b/lib/cartodb/models/dataview/formula.js @@ -112,8 +112,8 @@ module.exports = class Formula extends BaseDataview { const { result = 0, nulls_count = 0, - nans_count = 0, - infinities_count = 0 + nans_count, + infinities_count } = res.rows[0] || {}; return { From 222cfb90fdbc949160656b7a75dde1e44f5250c7 Mon Sep 17 00:00:00 2001 From: Simon Date: Mon, 18 Sep 2017 12:20:59 +0200 Subject: [PATCH 20/21] Removing 'self' vars using arrow functions --- lib/cartodb/models/dataview/formula.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/cartodb/models/dataview/formula.js b/lib/cartodb/models/dataview/formula.js index c0829fa5..daaeaf0c 100644 --- a/lib/cartodb/models/dataview/formula.js +++ b/lib/cartodb/models/dataview/formula.js @@ -78,8 +78,6 @@ module.exports = class Formula extends BaseDataview { sql (psql, override, callback) { - const self = this; - if (!callback) { callback = override; override = {}; @@ -87,11 +85,11 @@ module.exports = class Formula extends BaseDataview { if (this._isFloatColumn === null) { this._isFloatColumn = false; - this.getColumnType(psql, this.column, this.queries.no_filters, function (err, type) { + this.getColumnType(psql, this.column, this.queries.no_filters, (err, type) => { if (!err && !!type) { - self._isFloatColumn = type.float; + this._isFloatColumn = type.float; } - self.sql(psql, override, callback); + this.sql(psql, override, callback); }); return null; } From 8fe31c45f34b53ff1e22a1f20ebf6da66e31d5c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Mart=C3=ADn?= Date: Wed, 4 Oct 2017 11:10:17 +0200 Subject: [PATCH 21/21] fix 'this' scope with arrow function --- lib/cartodb/models/dataview/base.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/cartodb/models/dataview/base.js b/lib/cartodb/models/dataview/base.js index ea7f9474..8c1ada5d 100644 --- a/lib/cartodb/models/dataview/base.js +++ b/lib/cartodb/models/dataview/base.js @@ -26,7 +26,7 @@ module.exports = class BaseDataview { return callback(err); } - psql.query(query, function(err, result) { + psql.query(query, (err, result) => { if (err) { return callback(err, result); } @@ -48,7 +48,7 @@ module.exports = class BaseDataview { const readOnlyTransaction = true; const columnTypeQuery = columnTypeQueryTpl({ column, query }); - psql.query(columnTypeQuery, function(err, result) { + psql.query(columnTypeQuery, (err, result) => { if (err) { return callback(err); }