From 3850bbb68e4684317379ebaeee0f016d8ff082bc Mon Sep 17 00:00:00 2001 From: IagoLast Date: Thu, 31 May 2018 12:41:34 +0200 Subject: [PATCH 01/28] Send dates as unix epoch instead strings in .mvt files This commit creates a new ConfigAdapter used in vector maps instantiations. This adapter generates a new sql query for ONE SINGLE LAYER (carto-vl currently only supports one layer per mvt) where the date columns are wrapped into a epoch using the `date_part` function. Due this mvt files are smaller since we use numbers instead strings to represent dates, this is also faster in carto-gl where we interpolate linearly between 0 and 1 to create animations. Notice we should add a parameter to make this transformation optional. We also should take into account the epoch precission. --- lib/cartodb/api/api-router.js | 2 + .../dataview/histograms/date-histogram.js | 1 + .../adapter/vector-mapconfig-adapter.js | 39 ++++++++++++ lib/cartodb/utils/get-column-types.js | 60 +++++++++++++++++++ lib/cartodb/utils/query-utils.js | 50 ++++++++-------- 5 files changed, 128 insertions(+), 24 deletions(-) create mode 100644 lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js create mode 100644 lib/cartodb/utils/get-column-types.js diff --git a/lib/cartodb/api/api-router.js b/lib/cartodb/api/api-router.js index b0f42f03..6083ab0c 100644 --- a/lib/cartodb/api/api-router.js +++ b/lib/cartodb/api/api-router.js @@ -35,6 +35,7 @@ const TurboCartoAdapter = require('../models/mapconfig/adapter/turbo-carto-adapt const DataviewsWidgetsAdapter = require('../models/mapconfig/adapter/dataviews-widgets-adapter'); const AggregationMapConfigAdapter = require('../models/mapconfig/adapter/aggregation-mapconfig-adapter'); const MapConfigAdapter = require('../models/mapconfig/adapter'); +const VectorMapConfigAdapter = require('../models/mapconfig/adapter/vector-mapconfig-adapter'); const ResourceLocator = require('../models/resource-locator'); const LayergroupMetadata = require('../utils/layergroup-metadata'); @@ -135,6 +136,7 @@ module.exports = class ApiRouter { new SqlWrapMapConfigAdapter(), new DataviewsWidgetsAdapter(), new AnalysisMapConfigAdapter(analysisBackend), + new VectorMapConfigAdapter(pgConnection), new AggregationMapConfigAdapter(pgConnection), new MapConfigOverviewsAdapter(overviewsMetadataBackend, filterStatsBackend), new TurboCartoAdapter() diff --git a/lib/cartodb/models/dataview/histograms/date-histogram.js b/lib/cartodb/models/dataview/histograms/date-histogram.js index 4c720b3b..86e0e539 100644 --- a/lib/cartodb/models/dataview/histograms/date-histogram.js +++ b/lib/cartodb/models/dataview/histograms/date-histogram.js @@ -2,6 +2,7 @@ const BaseHistogram = require('./base-histogram'); const debug = require('debug')('windshaft:dataview:date-histogram'); const utils = require('../../../utils/query-utils'); + /** * Gets the name of a timezone with the same offset as the required * using the pg_timezone_names table. We do this because it's simpler to pass diff --git a/lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js b/lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js new file mode 100644 index 00000000..de88e3e0 --- /dev/null +++ b/lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js @@ -0,0 +1,39 @@ +const AggregationMapConfig = require('../../aggregation/aggregation-mapconfig'); +const utilsService = require('../../../utils/get-column-types'); + +// Generate query to detect time columns +// For every column cast to unix timestamp +module.exports = class VectorMapConfigAdapter { + constructor(pgConnection) { + this.pgConnection = pgConnection; + } + + getMapConfig(user, requestMapConfig, params, context, callback) { + let mapConfig; + try { + mapConfig = new AggregationMapConfig(user, requestMapConfig, this.pgConnection); + } catch (err) { + return callback(err); + } + + if (!mapConfig.isVectorOnlyMapConfig()) { + return callback(null, requestMapConfig); + } + + if (requestMapConfig.layers.lenght > 1) { + return callback(new Error('Get column types for multiple vector layers is not implemented')); + } + + + // // Get columns + utilsService.getColumns(user, this.pgConnection, requestMapConfig.layers[0]) + .then(result => { + const newSqlQuery = utilsService.wrapDates(requestMapConfig.layers[0].options.sql, result.fields); + requestMapConfig.layers[0].options.sql = newSqlQuery; + return callback(null, requestMapConfig); + }) + .catch(err => { + return callback(err); + }); + } +}; diff --git a/lib/cartodb/utils/get-column-types.js b/lib/cartodb/utils/get-column-types.js new file mode 100644 index 00000000..43991b26 --- /dev/null +++ b/lib/cartodb/utils/get-column-types.js @@ -0,0 +1,60 @@ +// Postgress ID of date types +const DATE_OIDS = { + 1082: true, + 1114: true, + 1184: true +}; + + +/** + * Wrap a query transforming all date columns into a unix epoch + * @param {*} originalQuery + * @param {*} fields + */ +function wrapDates(originalQuery, fields) { + return ` + SELECT + ${fields.map(field => DATE_OIDS.hasOwnProperty(field.dataTypeID) ? _castColumnToEpoch(field.name) : `${field.name}`).join(',')} + FROM + (${originalQuery}) _cdb_epoch_transformation `; +} + +/** + * Return a list of all the columns in the query + * @param {*} dbConnection + * @param {*} originalQuery + */ +function getColumns(user, dbConnection, layer) { + return _getColumns(user, dbConnection, layer.options.sql); +} + +/** + * Return a sql query to transform a date column into a unix epoch + * @param {string} column - The name of the date column + */ +function _castColumnToEpoch(columnName) { + return `date_part('epoch', ${columnName}) as ${columnName}`; +} + +function _getColumns(user, dbConnection, originalQuery) { + return new Promise((resolve, reject) => { + + dbConnection.getConnection(user, (err, connection) => { + if (err) { + return reject(err); + } + connection.query(`SELECT * FROM (${originalQuery}) _cdb_column_type limit 0`, (err, res) => { + if (err) { + return reject(err); + } + resolve(res); + }); + }); + }); +} + + +module.exports = { + wrapDates, + getColumns, +}; \ No newline at end of file diff --git a/lib/cartodb/utils/query-utils.js b/lib/cartodb/utils/query-utils.js index 867db0cd..c00e9dd5 100644 --- a/lib/cartodb/utils/query-utils.js +++ b/lib/cartodb/utils/query-utils.js @@ -1,16 +1,16 @@ function prepareQuery(sql) { - var affectedTableRegexCache = { - bbox: /!bbox!/g, - scale_denominator: /!scale_denominator!/g, - pixel_width: /!pixel_width!/g, - pixel_height: /!pixel_height!/g - }; + var affectedTableRegexCache = { + bbox: /!bbox!/g, + scale_denominator: /!scale_denominator!/g, + pixel_width: /!pixel_width!/g, + pixel_height: /!pixel_height!/g + }; - return sql - .replace(affectedTableRegexCache.bbox, 'ST_MakeEnvelope(0,0,0,0)') - .replace(affectedTableRegexCache.scale_denominator, '0') - .replace(affectedTableRegexCache.pixel_width, '1') - .replace(affectedTableRegexCache.pixel_height, '1'); + return sql + .replace(affectedTableRegexCache.bbox, 'ST_MakeEnvelope(0,0,0,0)') + .replace(affectedTableRegexCache.scale_denominator, '0') + .replace(affectedTableRegexCache.pixel_width, '1') + .replace(affectedTableRegexCache.pixel_height, '1'); } module.exports.extractTableNames = function extractTableNames(query) { @@ -59,25 +59,25 @@ module.exports.handleFloatColumn = function handleFloatColumn(ctx) { }; /** Count NULL appearances */ -module.exports.countNULLs= function countNULLs(ctx) { +module.exports.countNULLs = function countNULLs(ctx) { return `sum(CASE WHEN (${ctx.column} IS NULL) THEN 1 ELSE 0 END)`; }; /** Count only infinity (positive and negative) appearances */ module.exports.countInfinites = function countInfinites(ctx) { - return `${!ctx.isFloatColumn ? `0` : + return `${!ctx.isFloatColumn ? '0' : `sum(CASE WHEN (${ctx.column} = 'infinity'::float OR ${ctx.column} = '-infinity'::float) THEN 1 ELSE 0 END)` }`; }; /** Count only NaNs appearances*/ module.exports.countNaNs = function countNaNs(ctx) { - return `${!ctx.isFloatColumn ? `0` : + return `${!ctx.isFloatColumn ? '0' : `sum(CASE WHEN (${ctx.column} = 'NaN'::float) THEN 1 ELSE 0 END)` }`; }; -module.exports.getQueryTopCategories = function(query, column, topN, includeNulls=false) { +module.exports.getQueryTopCategories = function (query, column, topN, includeNulls = false) { const where = includeNulls ? '' : `WHERE ${column} IS NOT NULL`; return ` SELECT ${column} AS category, COUNT(*) AS frequency @@ -101,7 +101,7 @@ function columnSelector(columns) { throw new TypeError(`Bad argument type for columns: ${typeof columns}`); } -module.exports.getQuerySample = function(query, sampleProb, limit = null, randomSeed = 0.5, columns = null) { +module.exports.getQuerySample = function (query, sampleProb, limit = null, randomSeed = 0.5, columns = null) { const singleTable = simpleQueryTable(query); if (singleTable) { return getTableSample(singleTable.table, columns || singleTable.columns, sampleProb, limit, randomSeed); @@ -121,7 +121,7 @@ module.exports.getQuerySample = function(query, sampleProb, limit = null, random function getTableSample(table, columns, sampleProb, limit = null, randomSeed = 0.5) { const limitClause = limit ? `LIMIT ${limit}` : ''; sampleProb *= 100; - randomSeed *= Math.pow(2, 31) -1; + randomSeed *= Math.pow(2, 31) - 1; return ` SELECT ${columnSelector(columns)} FROM ${table} @@ -132,12 +132,12 @@ function getTableSample(table, columns, sampleProb, limit = null, randomSeed = 0 function simpleQueryTable(sql) { const basicQuery = /\s*SELECT\s+([\*a-z0-9_,\s]+?)\s+FROM\s+((\"[^"]+\"|[a-z0-9_]+)\.)?(\"[^"]+\"|[a-z0-9_]+)\s*;?\s*/i; - const unwrappedQuery = new RegExp("^"+basicQuery.source+"$", 'i'); + const unwrappedQuery = new RegExp('^' + basicQuery.source + '$', 'i'); // queries for named maps are wrapped like this: var wrappedQuery = new RegExp( - "^\\s*SELECT\\s+\\*\\s+FROM\\s+\\(" + + '^\\s*SELECT\\s+\\*\\s+FROM\\s+\\(' + basicQuery.source + - "\\)\\s+AS\\s+wrapped_query\\s+WHERE\\s+\\d+=1\\s*$", + '\\)\\s+AS\\s+wrapped_query\\s+WHERE\\s+\\d+=1\\s*$', 'i' ); let match = sql.match(unwrappedQuery); @@ -147,13 +147,13 @@ function simpleQueryTable(sql) { if (match) { const columns = match[1]; const schema = match[3]; - const table = match[4]; + const table = match[4]; return { table: schema ? `${schema}.${table}` : table, columns }; } return false; } -module.exports.getQueryGeometryType = function(query, geometryColumn) { +module.exports.getQueryGeometryType = function (query, geometryColumn) { return ` SELECT ST_GeometryType(${geometryColumn}) AS geom_type FROM (${query}) AS __cdb_query @@ -162,10 +162,12 @@ module.exports.getQueryGeometryType = function(query, geometryColumn) { `; }; -module.exports.getQueryLimited = function(query, limit=0) { +function getQueryLimited(query, limit = 0) { return ` SELECT * FROM (${query}) AS __cdb_query LIMIT ${limit} `; -}; +} + +module.exports.getQueryLimited = getQueryLimited; \ No newline at end of file From 4213e3163a46a99266bc8d4fdf6aa94bbb474ec5 Mon Sep 17 00:00:00 2001 From: elenatorro Date: Thu, 31 May 2018 18:37:43 +0200 Subject: [PATCH 02/28] Move queryPromise function --- .../layer-stats/mapnik-layer-stats.js | 22 +++++++------------ lib/cartodb/utils/query-utils.js | 7 ++++++ 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js b/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js index d8320334..64e28652 100644 --- a/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js +++ b/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js @@ -34,12 +34,6 @@ MapnikLayerStats.prototype.is = function (type) { return this._types[type] ? this._types[type] : false; }; -function queryPromise(dbConnection, query) { - return new Promise((resolve, reject) => { - dbConnection.query(query, (err, res) => err ? reject(err) : resolve(res)); - }); - } - function columnAggregations(field) { if (field.type === 'number') { return ['min', 'max', 'avg', 'sum']; @@ -63,7 +57,7 @@ function _getSQL(ctx, query, type='pre', zoom=0) { } function _estimatedFeatureCount(ctx) { - return queryPromise(ctx.dbConnection, _getSQL(ctx, queryUtils.getQueryRowEstimation)) + return queryUtils.queryPromise(ctx.dbConnection, _getSQL(ctx, queryUtils.getQueryRowEstimation)) .then(res => ({ estimatedFeatureCount: res.rows[0].rows })) .catch(() => ({ estimatedFeatureCount: -1 })); } @@ -71,7 +65,7 @@ function _estimatedFeatureCount(ctx) { function _featureCount(ctx) { if (ctx.metaOptions.featureCount) { // TODO: if ctx.metaOptions.columnStats we can combine this with column stats query - return queryPromise(ctx.dbConnection, _getSQL(ctx, queryUtils.getQueryActualRowCount)) + return queryUtils.queryPromise(ctx.dbConnection, _getSQL(ctx, queryUtils.getQueryActualRowCount)) .then(res => ({ featureCount: res.rows[0].rows })); } return Promise.resolve(); @@ -82,7 +76,7 @@ function _aggrFeatureCount(ctx) { // We expect as zoom level as the value of aggrFeatureCount // TODO: it'd be nice to admit an array of zoom levels to // return metadata for multiple levels. - return queryPromise( + return queryUtils.queryPromise( ctx.dbConnection, _getSQL(ctx, queryUtils.getQueryActualRowCount, 'post', ctx.metaOptions.aggrFeatureCount) ).then(res => ({ aggrFeatureCount: res.rows[0].rows })); @@ -93,7 +87,7 @@ function _aggrFeatureCount(ctx) { function _geometryType(ctx) { if (ctx.metaOptions.geometryType) { const geometryColumn = AggregationMapConfig.getAggregationGeometryColumn(); - return queryPromise(ctx.dbConnection, _getSQL(ctx, sql => queryUtils.getQueryGeometryType(sql, geometryColumn))) + return queryUtils.queryPromise(ctx.dbConnection, _getSQL(ctx, sql => queryUtils.getQueryGeometryType(sql, geometryColumn))) .then(res => ({ geometryType: res.rows[0].geom_type })); } return Promise.resolve(); @@ -102,7 +96,7 @@ function _geometryType(ctx) { function _columns(ctx) { if (ctx.metaOptions.columns || ctx.metaOptions.columnStats) { // note: post-aggregation columns are in layer.options.columns when aggregation is present - return queryPromise(ctx.dbConnection, _getSQL(ctx, sql => queryUtils.getQueryLimited(sql, 0))) + return queryUtils.queryPromise(ctx.dbConnection, _getSQL(ctx, sql => queryUtils.getQueryLimited(sql, 0))) .then(res => formatResultFields(ctx.dbConnection, res.fields)); } return Promise.resolve(); @@ -156,7 +150,7 @@ function _sample(ctx, numRows) { const requestedRows = ctx.metaOptions.sample.num_rows || DEFAULT_SAMPLE_ROWS; const limit = Math.ceil(requestedRows * 1.5); let columns = ctx.metaOptions.sample.include_columns; - return queryPromise(ctx.dbConnection, _getSQL( + return queryUtils.queryPromise(ctx.dbConnection, _getSQL( ctx, sql => queryUtils.getQuerySample(sql, sampleProb, limit, SAMPLE_SEED, columns) )).then(res => ({ sample: res.rows })); @@ -186,7 +180,7 @@ function _columnStats(ctx, columns) { // TODO: ctx.metaOptions.columnStats.maxCategories // => use PG stats to dismiss columns with more distinct values queries.push( - queryPromise( + queryUtils.queryPromise( ctx.dbConnection, _getSQL(ctx, sql => queryUtils.getQueryTopCategories(sql, name, topN, includeNulls)) ).then(res => ({ [name]: { categories: res.rows } })) @@ -194,7 +188,7 @@ function _columnStats(ctx, columns) { } }); queries.push( - queryPromise( + queryUtils.queryPromise( ctx.dbConnection, _getSQL(ctx, sql => `SELECT ${aggr.join(',')} FROM (${sql}) AS __cdb_query`) ).then(res => { diff --git a/lib/cartodb/utils/query-utils.js b/lib/cartodb/utils/query-utils.js index c00e9dd5..ed239422 100644 --- a/lib/cartodb/utils/query-utils.js +++ b/lib/cartodb/utils/query-utils.js @@ -170,4 +170,11 @@ function getQueryLimited(query, limit = 0) { `; } +function queryPromise(dbConnection, query) { + return new Promise((resolve, reject) => { + dbConnection.query(query, (err, res) => err ? reject(err) : resolve(res)); + }); +} + +module.exports.queryPromise = queryPromise; module.exports.getQueryLimited = getQueryLimited; \ No newline at end of file From db3370cd21ec6db6ce23fd6fe6b8f499565f85f4 Mon Sep 17 00:00:00 2001 From: elenatorro Date: Thu, 31 May 2018 18:46:23 +0200 Subject: [PATCH 03/28] Use wrapDates function from vector adapter --- .../adapter/vector-mapconfig-adapter.js | 35 +++++++++++---- lib/cartodb/utils/get-column-types.js | 45 +++++-------------- 2 files changed, 39 insertions(+), 41 deletions(-) diff --git a/lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js b/lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js index de88e3e0..05d08704 100644 --- a/lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js +++ b/lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js @@ -1,5 +1,6 @@ const AggregationMapConfig = require('../../aggregation/aggregation-mapconfig'); const utilsService = require('../../../utils/get-column-types'); +const queryUtils = require('../../../utils/query-utils'); // Generate query to detect time columns // For every column cast to unix timestamp @@ -25,15 +26,33 @@ module.exports = class VectorMapConfigAdapter { } - // // Get columns - utilsService.getColumns(user, this.pgConnection, requestMapConfig.layers[0]) + this._wrapDates(requestMapConfig, user) + .then(updatedRequestMapConfig => callback(null, updatedRequestMapConfig)) + .catch(callback); + } + + _wrapDates(requestMapConfig, user) { + const originalQuery = requestMapConfig.layers[0].options.sql; + return this._getColumns(user, originalQuery) .then(result => { - const newSqlQuery = utilsService.wrapDates(requestMapConfig.layers[0].options.sql, result.fields); + const newSqlQuery = utilsService.wrapDates(originalQuery, result.fields); requestMapConfig.layers[0].options.sql = newSqlQuery; - return callback(null, requestMapConfig); - }) - .catch(err => { - return callback(err); + return requestMapConfig; }); } -}; + + _getColumns(user, originalQuery) { + return new Promise((resolve, reject) => { + this.pgConnection.getConnection(user, (err, connection) => { + if (err) { + return reject(err); + } + const query = `SELECT * FROM (${originalQuery}) _cdb_column_type limit 0`; + queryUtils.queryPromise(connection, query) + .then(resolve) + .catch(reject); + // TODO release pgConnection + }); + }); + } +}; \ No newline at end of file diff --git a/lib/cartodb/utils/get-column-types.js b/lib/cartodb/utils/get-column-types.js index 43991b26..d301026d 100644 --- a/lib/cartodb/utils/get-column-types.js +++ b/lib/cartodb/utils/get-column-types.js @@ -1,10 +1,10 @@ -// Postgress ID of date types -const DATE_OIDS = { - 1082: true, - 1114: true, - 1184: true -}; - +const DATE_OIDS = Object.freeze({ + 1082: 'DATE', + 1083: 'TIME', + 1114: 'TIMESTAMP', + 1184: 'TIMESTAMPTZ', + 1266: 'TIMETZ' +}); /** * Wrap a query transforming all date columns into a unix epoch @@ -14,18 +14,16 @@ const DATE_OIDS = { function wrapDates(originalQuery, fields) { return ` SELECT - ${fields.map(field => DATE_OIDS.hasOwnProperty(field.dataTypeID) ? _castColumnToEpoch(field.name) : `${field.name}`).join(',')} + ${fields.map(field => _isDateType(field) ? _castColumnToEpoch(field.name) : `${field.name}`).join(',')} FROM (${originalQuery}) _cdb_epoch_transformation `; } /** - * Return a list of all the columns in the query - * @param {*} dbConnection - * @param {*} originalQuery + * @param {object} field */ -function getColumns(user, dbConnection, layer) { - return _getColumns(user, dbConnection, layer.options.sql); +function _isDateType(field) { + return DATE_OIDS.hasOwnProperty(field.dataTypeID); } /** @@ -36,25 +34,6 @@ function _castColumnToEpoch(columnName) { return `date_part('epoch', ${columnName}) as ${columnName}`; } -function _getColumns(user, dbConnection, originalQuery) { - return new Promise((resolve, reject) => { - - dbConnection.getConnection(user, (err, connection) => { - if (err) { - return reject(err); - } - connection.query(`SELECT * FROM (${originalQuery}) _cdb_column_type limit 0`, (err, res) => { - if (err) { - return reject(err); - } - resolve(res); - }); - }); - }); -} - - module.exports = { - wrapDates, - getColumns, + wrapDates }; \ No newline at end of file From d4c62824553a684a529bc85465900d2460f6b0f5 Mon Sep 17 00:00:00 2001 From: elenatorro Date: Thu, 31 May 2018 18:53:01 +0200 Subject: [PATCH 04/28] Refactor date wrapper --- .../models/mapconfig/adapter/vector-mapconfig-adapter.js | 4 ++-- lib/cartodb/utils/{get-column-types.js => date-wrapper.js} | 0 2 files changed, 2 insertions(+), 2 deletions(-) rename lib/cartodb/utils/{get-column-types.js => date-wrapper.js} (100%) diff --git a/lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js b/lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js index 05d08704..e1e4cc4a 100644 --- a/lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js +++ b/lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js @@ -1,5 +1,5 @@ const AggregationMapConfig = require('../../aggregation/aggregation-mapconfig'); -const utilsService = require('../../../utils/get-column-types'); +const dateWrapper = require('../../../utils/date-wrapper'); const queryUtils = require('../../../utils/query-utils'); // Generate query to detect time columns @@ -35,7 +35,7 @@ module.exports = class VectorMapConfigAdapter { const originalQuery = requestMapConfig.layers[0].options.sql; return this._getColumns(user, originalQuery) .then(result => { - const newSqlQuery = utilsService.wrapDates(originalQuery, result.fields); + const newSqlQuery = dateWrapper.wrapDates(originalQuery, result.fields); requestMapConfig.layers[0].options.sql = newSqlQuery; return requestMapConfig; }); diff --git a/lib/cartodb/utils/get-column-types.js b/lib/cartodb/utils/date-wrapper.js similarity index 100% rename from lib/cartodb/utils/get-column-types.js rename to lib/cartodb/utils/date-wrapper.js From 79962a7566c8b0fcae84fc6734840c1a726f2766 Mon Sep 17 00:00:00 2001 From: elenatorro Date: Thu, 31 May 2018 19:07:57 +0200 Subject: [PATCH 05/28] Refactor long line --- lib/cartodb/backends/layer-stats/mapnik-layer-stats.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js b/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js index 64e28652..d5f841b8 100644 --- a/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js +++ b/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js @@ -87,7 +87,8 @@ function _aggrFeatureCount(ctx) { function _geometryType(ctx) { if (ctx.metaOptions.geometryType) { const geometryColumn = AggregationMapConfig.getAggregationGeometryColumn(); - return queryUtils.queryPromise(ctx.dbConnection, _getSQL(ctx, sql => queryUtils.getQueryGeometryType(sql, geometryColumn))) + const sqlQuery = _getSQL(ctx, sql => queryUtils.getQueryGeometryType(sql, geometryColumn)); + return queryUtils.queryPromise(ctx.dbConnection, sqlQuery) .then(res => ({ geometryType: res.rows[0].geom_type })); } return Promise.resolve(); From 6872d575813c3c1eee9635a5079d7f609d4243dc Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Fri, 1 Jun 2018 10:08:34 +0000 Subject: [PATCH 06/28] Add tests --- test/acceptance/date-wrapping.spec.js | 74 ++++++++++++++++++++++++++ test/fixtures/test_mapconfigFactory.js | 51 ++++++++++++++++++ 2 files changed, 125 insertions(+) create mode 100644 test/acceptance/date-wrapping.spec.js create mode 100644 test/fixtures/test_mapconfigFactory.js diff --git a/test/acceptance/date-wrapping.spec.js b/test/acceptance/date-wrapping.spec.js new file mode 100644 index 00000000..5b8953a0 --- /dev/null +++ b/test/acceptance/date-wrapping.spec.js @@ -0,0 +1,74 @@ +/* eslint-env mocha */ +const assert = require('assert'); +const TestClient = require('../support/test-client'); +const mapConfigFactory = require('../fixtures/test_mapconfigFactory'); + +describe.only('date-wrapping', () => { + let testClient; + + describe('when a map instantiation has the "dates_as_numbers" option enabled', () => { + beforeEach(() => { + const mapConfig = mapConfigFactory.getVectorMapConfig({ dates_as_numbers: true }); + testClient = new TestClient(mapConfig); + }); + + afterEach(done => testClient.drain(done)); + + it('should return date columns casted as numbers', done => { + + testClient.getTile(0, 0, 0, { format: 'mvt' }, (err, res, mvt) => { + const expected = [ + { + type: 'Feature', + id: 1, + geometry: { type: 'Point', coordinates: [0, 0] }, + properties: { _cdb_feature_count: 1, cartodb_id: 0, date: 1527810000 } + }, + { + type: 'Feature', + id: 2, + geometry: { type: 'Point', coordinates: [0, 0] }, + properties: { _cdb_feature_count: 1, cartodb_id: 1, date: 1527900000 } + } + ]; + const actual = JSON.parse(mvt.toGeoJSONSync(0)).features; + + assert.deepEqual(actual, expected); + done(); + }); + }); + }); + + describe('when a map instantiation has the "dates_as_numbers" option disabled', () => { + beforeEach(() => { + const mapConfig = mapConfigFactory.getVectorMapConfig({ dates_as_numbers: false }); + testClient = new TestClient(mapConfig); + }); + + afterEach(done => testClient.drain(done)); + + it('should return date columns as dates', done => { + + testClient.getTile(0, 0, 0, { format: 'mvt' }, (err, res, mvt) => { + const expected = [ + { + type: 'Feature', + id: 1, + geometry: { type: 'Point', coordinates: [0, 0] }, + properties: { _cdb_feature_count: 1, cartodb_id: 0 } + }, + { + type: 'Feature', + id: 2, + geometry: { type: 'Point', coordinates: [0, 0] }, + properties: { _cdb_feature_count: 1, cartodb_id: 1 } + } + ]; + const actual = JSON.parse(mvt.toGeoJSONSync(0)).features; + + assert.deepEqual(actual, expected); + done(); + }); + }); + }); +}); \ No newline at end of file diff --git a/test/fixtures/test_mapconfigFactory.js b/test/fixtures/test_mapconfigFactory.js new file mode 100644 index 00000000..54643475 --- /dev/null +++ b/test/fixtures/test_mapconfigFactory.js @@ -0,0 +1,51 @@ + +function getVectorMapConfig(opts) { + return { + buffersize: { + mvt: 1 + }, + layers: [ + { + type: 'mapnik', + options: { + sql: ` + SELECT + (DATE '2018-06-01' + x) as date, + x as cartodb_id, + st_makepoint(x * 10, x * 10) as the_geom, + st_makepoint(x * 10, x * 10) as the_geom_webmercator + FROM + generate_series(0, 1) x`, + aggregation: { + columns: {}, + dimensions: { + date: 'date' + }, + placement: 'centroid', + resolution: 1, + threshold: 1 + }, + dates_as_numbers: opts.dates_as_numbers, + metadata: { + geometryType: true, + columnStats: { + topCategories: 32768, + includeNulls: true + }, + sample: { + num_rows: 1000, + include_columns: [ + 'date' + ] + } + } + } + } + ] + }; +} + + + + +module.exports = { getVectorMapConfig }; \ No newline at end of file From ae4b233458b1f6ce8775a642de8c228972e5edde Mon Sep 17 00:00:00 2001 From: Ubuntu Date: Fri, 1 Jun 2018 10:18:07 +0000 Subject: [PATCH 07/28] Pass tests --- .../adapter/vector-mapconfig-adapter.js | 33 ++++++++++--------- test/acceptance/date-wrapping.spec.js | 2 +- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js b/lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js index e1e4cc4a..76fdd836 100644 --- a/lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js +++ b/lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js @@ -1,23 +1,20 @@ -const AggregationMapConfig = require('../../aggregation/aggregation-mapconfig'); -const dateWrapper = require('../../../utils/date-wrapper'); const queryUtils = require('../../../utils/query-utils'); +const dateWrapper = require('../../../utils/date-wrapper'); -// Generate query to detect time columns -// For every column cast to unix timestamp -module.exports = class VectorMapConfigAdapter { +/** + * This middleware wraps the layer query transforming the date fields into numbers because mvt tiles + * doesnt support dates as primitive type. + * + * - This middleware is ONLY activated when the `dates_as_numbers` option is enabled for some layer in the mapConfig. + * - TODO: We currently support one single layer and we should define what to do with multiple layers. + */ +class VectorMapConfigAdapter { constructor(pgConnection) { this.pgConnection = pgConnection; } getMapConfig(user, requestMapConfig, params, context, callback) { - let mapConfig; - try { - mapConfig = new AggregationMapConfig(user, requestMapConfig, this.pgConnection); - } catch (err) { - return callback(err); - } - - if (!mapConfig.isVectorOnlyMapConfig()) { + if (!this._isDatesAsNumbersFlagEnabled(requestMapConfig)) { return callback(null, requestMapConfig); } @@ -25,7 +22,6 @@ module.exports = class VectorMapConfigAdapter { return callback(new Error('Get column types for multiple vector layers is not implemented')); } - this._wrapDates(requestMapConfig, user) .then(updatedRequestMapConfig => callback(null, updatedRequestMapConfig)) .catch(callback); @@ -55,4 +51,11 @@ module.exports = class VectorMapConfigAdapter { }); }); } -}; \ No newline at end of file + + _isDatesAsNumbersFlagEnabled(requestMapConfig) { + return requestMapConfig.layers.some(layer => layer.options.dates_as_numbers); + } +} + + +module.exports = VectorMapConfigAdapter; \ No newline at end of file diff --git a/test/acceptance/date-wrapping.spec.js b/test/acceptance/date-wrapping.spec.js index 5b8953a0..5049fd51 100644 --- a/test/acceptance/date-wrapping.spec.js +++ b/test/acceptance/date-wrapping.spec.js @@ -3,7 +3,7 @@ const assert = require('assert'); const TestClient = require('../support/test-client'); const mapConfigFactory = require('../fixtures/test_mapconfigFactory'); -describe.only('date-wrapping', () => { +describe('date-wrapping', () => { let testClient; describe('when a map instantiation has the "dates_as_numbers" option enabled', () => { From 8ec2b35557914c7f109f0209cb402e96d9631653 Mon Sep 17 00:00:00 2001 From: IagoLast Date: Fri, 1 Jun 2018 12:25:36 +0200 Subject: [PATCH 08/28] Fix tests --- .../models/mapconfig/adapter/vector-mapconfig-adapter.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js b/lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js index 76fdd836..4e5020a6 100644 --- a/lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js +++ b/lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js @@ -53,7 +53,7 @@ class VectorMapConfigAdapter { } _isDatesAsNumbersFlagEnabled(requestMapConfig) { - return requestMapConfig.layers.some(layer => layer.options.dates_as_numbers); + return requestMapConfig.layers && requestMapConfig.layers.some(layer => layer.options.dates_as_numbers); } } From 2ee6c8487d0d1dc2167b9a572307d6aade63cbbe Mon Sep 17 00:00:00 2001 From: IagoLast Date: Tue, 5 Jun 2018 08:44:20 +0200 Subject: [PATCH 09/28] PR style fixes --- .../models/mapconfig/adapter/vector-mapconfig-adapter.js | 5 ++--- lib/cartodb/utils/date-wrapper.js | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js b/lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js index 4e5020a6..14c971e9 100644 --- a/lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js +++ b/lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js @@ -18,7 +18,7 @@ class VectorMapConfigAdapter { return callback(null, requestMapConfig); } - if (requestMapConfig.layers.lenght > 1) { + if (requestMapConfig.layers.length > 1) { return callback(new Error('Get column types for multiple vector layers is not implemented')); } @@ -43,11 +43,10 @@ class VectorMapConfigAdapter { if (err) { return reject(err); } - const query = `SELECT * FROM (${originalQuery}) _cdb_column_type limit 0`; + const query = queryUtils.getQueryLimited(originalQuery, 0); queryUtils.queryPromise(connection, query) .then(resolve) .catch(reject); - // TODO release pgConnection }); }); } diff --git a/lib/cartodb/utils/date-wrapper.js b/lib/cartodb/utils/date-wrapper.js index d301026d..dfbf52dd 100644 --- a/lib/cartodb/utils/date-wrapper.js +++ b/lib/cartodb/utils/date-wrapper.js @@ -31,7 +31,7 @@ function _isDateType(field) { * @param {string} column - The name of the date column */ function _castColumnToEpoch(columnName) { - return `date_part('epoch', ${columnName}) as ${columnName}`; + return `date_part('epoch', ${columnName}) as "${columnName}"`; } module.exports = { From 7df1a19da4ec2bdaaf3d0923799ffe7694af5366 Mon Sep 17 00:00:00 2001 From: IagoLast Date: Tue, 5 Jun 2018 08:58:44 +0200 Subject: [PATCH 10/28] Add test for multiple-layer date wrap --- test/acceptance/date-wrapping.spec.js | 122 +++++++++++++++---------- test/fixtures/test_mapconfigFactory.js | 75 +++++++-------- 2 files changed, 112 insertions(+), 85 deletions(-) diff --git a/test/acceptance/date-wrapping.spec.js b/test/acceptance/date-wrapping.spec.js index 5049fd51..fed6278e 100644 --- a/test/acceptance/date-wrapping.spec.js +++ b/test/acceptance/date-wrapping.spec.js @@ -6,69 +6,93 @@ const mapConfigFactory = require('../fixtures/test_mapconfigFactory'); describe('date-wrapping', () => { let testClient; - describe('when a map instantiation has the "dates_as_numbers" option enabled', () => { - beforeEach(() => { - const mapConfig = mapConfigFactory.getVectorMapConfig({ dates_as_numbers: true }); - testClient = new TestClient(mapConfig); + describe('when a map instantiation has one single layer', () => { + describe('and the layer has the "dates_as_numbers" option enabled', () => { + beforeEach(() => { + const mapConfig = mapConfigFactory.getVectorMapConfig({ dates_as_numbers: true }); + testClient = new TestClient(mapConfig); + }); + + afterEach(done => testClient.drain(done)); + + it('should return date columns casted as numbers', done => { + + testClient.getTile(0, 0, 0, { format: 'mvt' }, (err, res, mvt) => { + const expected = [ + { + type: 'Feature', + id: 1, + geometry: { type: 'Point', coordinates: [0, 0] }, + properties: { _cdb_feature_count: 1, cartodb_id: 0, date: 1527810000 } + }, + { + type: 'Feature', + id: 2, + geometry: { type: 'Point', coordinates: [0, 0] }, + properties: { _cdb_feature_count: 1, cartodb_id: 1, date: 1527900000 } + } + ]; + const actual = JSON.parse(mvt.toGeoJSONSync(0)).features; + + assert.deepEqual(actual, expected); + done(); + }); + }); }); - afterEach(done => testClient.drain(done)); + describe('and the layer has the "dates_as_numbers" option disabled', () => { + beforeEach(() => { + const mapConfig = mapConfigFactory.getVectorMapConfig({ dates_as_numbers: false }); + testClient = new TestClient(mapConfig); + }); - it('should return date columns casted as numbers', done => { + afterEach(done => testClient.drain(done)); - testClient.getTile(0, 0, 0, { format: 'mvt' }, (err, res, mvt) => { - const expected = [ - { - type: 'Feature', - id: 1, - geometry: { type: 'Point', coordinates: [0, 0] }, - properties: { _cdb_feature_count: 1, cartodb_id: 0, date: 1527810000 } - }, - { - type: 'Feature', - id: 2, - geometry: { type: 'Point', coordinates: [0, 0] }, - properties: { _cdb_feature_count: 1, cartodb_id: 1, date: 1527900000 } - } - ]; - const actual = JSON.parse(mvt.toGeoJSONSync(0)).features; + it('should return date columns as dates', done => { - assert.deepEqual(actual, expected); - done(); + testClient.getTile(0, 0, 0, { format: 'mvt' }, (err, res, mvt) => { + const expected = [ + { + type: 'Feature', + id: 1, + geometry: { type: 'Point', coordinates: [0, 0] }, + properties: { _cdb_feature_count: 1, cartodb_id: 0 } + }, + { + type: 'Feature', + id: 2, + geometry: { type: 'Point', coordinates: [0, 0] }, + properties: { _cdb_feature_count: 1, cartodb_id: 1 } + } + ]; + const actual = JSON.parse(mvt.toGeoJSONSync(0)).features; + + assert.deepEqual(actual, expected); + done(); + }); }); }); }); - describe('when a map instantiation has the "dates_as_numbers" option disabled', () => { + + describe('when a map instantiation has multiple layers', () => { beforeEach(() => { - const mapConfig = mapConfigFactory.getVectorMapConfig({ dates_as_numbers: false }); + const mapConfig = mapConfigFactory.getVectorMapConfig({ numberOfLayers: 2 }); testClient = new TestClient(mapConfig); }); + describe('and both layers have the "dates_as_numbers" option enabled', () => { + // TODO: Pending test + it('should return dates as numbers for every layer'); + }); - afterEach(done => testClient.drain(done)); + describe('and only one layers has the "dates_as_numbers" option enabled', () => { + // TODO: Pending test + it('should return dates as numbers only for the layer with the "dates_as_numbers" flag enabled'); + }); - it('should return date columns as dates', done => { - - testClient.getTile(0, 0, 0, { format: 'mvt' }, (err, res, mvt) => { - const expected = [ - { - type: 'Feature', - id: 1, - geometry: { type: 'Point', coordinates: [0, 0] }, - properties: { _cdb_feature_count: 1, cartodb_id: 0 } - }, - { - type: 'Feature', - id: 2, - geometry: { type: 'Point', coordinates: [0, 0] }, - properties: { _cdb_feature_count: 1, cartodb_id: 1 } - } - ]; - const actual = JSON.parse(mvt.toGeoJSONSync(0)).features; - - assert.deepEqual(actual, expected); - done(); - }); + describe('and none of the layers has the "dates_as_numbers" option enabled', () => { + // TODO: Pending test + it('should return dates as dates for both layers'); }); }); }); \ No newline at end of file diff --git a/test/fixtures/test_mapconfigFactory.js b/test/fixtures/test_mapconfigFactory.js index 54643475..0a46de5f 100644 --- a/test/fixtures/test_mapconfigFactory.js +++ b/test/fixtures/test_mapconfigFactory.js @@ -4,44 +4,47 @@ function getVectorMapConfig(opts) { buffersize: { mvt: 1 }, - layers: [ - { - type: 'mapnik', - options: { - sql: ` - SELECT - (DATE '2018-06-01' + x) as date, - x as cartodb_id, - st_makepoint(x * 10, x * 10) as the_geom, - st_makepoint(x * 10, x * 10) as the_geom_webmercator - FROM - generate_series(0, 1) x`, - aggregation: { - columns: {}, - dimensions: { - date: 'date' - }, - placement: 'centroid', - resolution: 1, - threshold: 1 - }, - dates_as_numbers: opts.dates_as_numbers, - metadata: { - geometryType: true, - columnStats: { - topCategories: 32768, - includeNulls: true - }, - sample: { - num_rows: 1000, - include_columns: [ - 'date' - ] - } - } + layers: Array(opts.numberOfLayers ||  1).map(() => _generateLayerConfig(opts)); + }; +} + + +function _generateLayerConfig(opts) { + return { + type: 'mapnik', + options: { + sql: ` + SELECT + (DATE '2018-06-01' + x) as date, + x as cartodb_id, + st_makepoint(x * 10, x * 10) as the_geom, + st_makepoint(x * 10, x * 10) as the_geom_webmercator + FROM + generate_series(0, 1) x`, + aggregation: { + columns: {}, + dimensions: { + date: 'date' + }, + placement: 'centroid', + resolution: 1, + threshold: 1 + }, + dates_as_numbers: opts.dates_as_numbers, + metadata: { + geometryType: true, + columnStats: { + topCategories: 32768, + includeNulls: true + }, + sample: { + num_rows: 1000, + include_columns: [ + 'date' + ] } } - ] + } }; } From 9ee6d7fc91937337adfa99cb188a82194076a549 Mon Sep 17 00:00:00 2001 From: IagoLast Date: Tue, 5 Jun 2018 09:34:44 +0200 Subject: [PATCH 11/28] Implement multiple layer date wrapping --- .../mapconfig/adapter/vector-mapconfig-adapter.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js b/lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js index 14c971e9..3472a4be 100644 --- a/lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js +++ b/lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js @@ -28,12 +28,17 @@ class VectorMapConfigAdapter { } _wrapDates(requestMapConfig, user) { - const originalQuery = requestMapConfig.layers[0].options.sql; + return Promise.all(requestMapConfig.layers.map(layer => this._wrapLayer(layer, user))) + .then(() => requestMapConfig); + } + + _wrapLayer(layer, user) { + const originalQuery = layer.options.sql; return this._getColumns(user, originalQuery) .then(result => { const newSqlQuery = dateWrapper.wrapDates(originalQuery, result.fields); - requestMapConfig.layers[0].options.sql = newSqlQuery; - return requestMapConfig; + layer.options.sql = newSqlQuery; + return layer; }); } From a883514c8a0daf7898e8ab8c250bc61ddcdfb6e2 Mon Sep 17 00:00:00 2001 From: IagoLast Date: Tue, 5 Jun 2018 09:35:01 +0200 Subject: [PATCH 12/28] Remove control flag --- .../models/mapconfig/adapter/vector-mapconfig-adapter.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js b/lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js index 3472a4be..aada7b0f 100644 --- a/lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js +++ b/lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js @@ -6,7 +6,6 @@ const dateWrapper = require('../../../utils/date-wrapper'); * doesnt support dates as primitive type. * * - This middleware is ONLY activated when the `dates_as_numbers` option is enabled for some layer in the mapConfig. - * - TODO: We currently support one single layer and we should define what to do with multiple layers. */ class VectorMapConfigAdapter { constructor(pgConnection) { @@ -18,10 +17,6 @@ class VectorMapConfigAdapter { return callback(null, requestMapConfig); } - if (requestMapConfig.layers.length > 1) { - return callback(new Error('Get column types for multiple vector layers is not implemented')); - } - this._wrapDates(requestMapConfig, user) .then(updatedRequestMapConfig => callback(null, updatedRequestMapConfig)) .catch(callback); From 9f4b6d5f43ced09611ed36ee9f455514a52dd9fb Mon Sep 17 00:00:00 2001 From: IagoLast Date: Tue, 5 Jun 2018 09:57:48 +0200 Subject: [PATCH 13/28] Fix linter --- test/fixtures/test_mapconfigFactory.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/fixtures/test_mapconfigFactory.js b/test/fixtures/test_mapconfigFactory.js index 0a46de5f..6ccf2753 100644 --- a/test/fixtures/test_mapconfigFactory.js +++ b/test/fixtures/test_mapconfigFactory.js @@ -4,7 +4,7 @@ function getVectorMapConfig(opts) { buffersize: { mvt: 1 }, - layers: Array(opts.numberOfLayers ||  1).map(() => _generateLayerConfig(opts)); + layers: Array(opts.numberOfLayers || 1).map(() => _generateLayerConfig(opts)), }; } From 1491f29f96047538cf72aed9b6ceb05c03c96ddf Mon Sep 17 00:00:00 2001 From: IagoLast Date: Tue, 5 Jun 2018 10:10:56 +0200 Subject: [PATCH 14/28] Fix tests --- test/fixtures/test_mapconfigFactory.js | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/test/fixtures/test_mapconfigFactory.js b/test/fixtures/test_mapconfigFactory.js index 6ccf2753..4c71bbab 100644 --- a/test/fixtures/test_mapconfigFactory.js +++ b/test/fixtures/test_mapconfigFactory.js @@ -1,13 +1,20 @@ - function getVectorMapConfig(opts) { return { buffersize: { mvt: 1 }, - layers: Array(opts.numberOfLayers || 1).map(() => _generateLayerConfig(opts)), + layers: _generateLayers(opts), }; } +function _generateLayers(opts) { + const numberOfLayers = opts.numberOfLayers || 1; + const layers = []; + for (let index = 0; index <= numberOfLayers; index++) { + layers.push(_generateLayerConfig(opts)); + } + return layers; +} function _generateLayerConfig(opts) { return { From b10cf4bebb5e9c78e27aca70547db93b11f4b8a5 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Tue, 5 Jun 2018 11:25:00 +0200 Subject: [PATCH 15/28] New test for casted dates metadata --- test/acceptance/date-wrapping.spec.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/acceptance/date-wrapping.spec.js b/test/acceptance/date-wrapping.spec.js index fed6278e..cf8d6508 100644 --- a/test/acceptance/date-wrapping.spec.js +++ b/test/acceptance/date-wrapping.spec.js @@ -38,6 +38,18 @@ describe('date-wrapping', () => { done(); }); }); + + it('should return metadata with casted columns', done => { + + testClient.getLayergroup(function(err, layergroup) { + const expected = ['date']; + assert.ifError(err); + assert.deepEqual(layergroup.metadata.layers[0].meta.dates_as_numbers.sort(), expected.sort()); + testClient.drain(done); + }); + + }); + }); describe('and the layer has the "dates_as_numbers" option disabled', () => { From e8ecd9b2e00f3874ad9ee0d8ab7a032531590fa6 Mon Sep 17 00:00:00 2001 From: IagoLast Date: Tue, 5 Jun 2018 11:44:30 +0200 Subject: [PATCH 16/28] Fix new test --- test/acceptance/date-wrapping.spec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/acceptance/date-wrapping.spec.js b/test/acceptance/date-wrapping.spec.js index cf8d6508..f36e64db 100644 --- a/test/acceptance/date-wrapping.spec.js +++ b/test/acceptance/date-wrapping.spec.js @@ -6,7 +6,7 @@ const mapConfigFactory = require('../fixtures/test_mapconfigFactory'); describe('date-wrapping', () => { let testClient; - describe('when a map instantiation has one single layer', () => { + describe.only('when a map instantiation has one single layer', () => { describe('and the layer has the "dates_as_numbers" option enabled', () => { beforeEach(() => { const mapConfig = mapConfigFactory.getVectorMapConfig({ dates_as_numbers: true }); @@ -45,7 +45,7 @@ describe('date-wrapping', () => { const expected = ['date']; assert.ifError(err); assert.deepEqual(layergroup.metadata.layers[0].meta.dates_as_numbers.sort(), expected.sort()); - testClient.drain(done); + done(); }); }); From 55f62417692c977bade49fe1694b0e5ff690531d Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Tue, 5 Jun 2018 15:39:01 +0200 Subject: [PATCH 17/28] Add date wrapping metadata --- .../api/middlewares/layergroup-metadata.js | 1 + lib/cartodb/utils/date-wrapper.js | 32 ++++++++++++++----- lib/cartodb/utils/layergroup-metadata.js | 16 ++++++++++ 3 files changed, 41 insertions(+), 8 deletions(-) diff --git a/lib/cartodb/api/middlewares/layergroup-metadata.js b/lib/cartodb/api/middlewares/layergroup-metadata.js index 301089df..bbb1fbe4 100644 --- a/lib/cartodb/api/middlewares/layergroup-metadata.js +++ b/lib/cartodb/api/middlewares/layergroup-metadata.js @@ -7,6 +7,7 @@ module.exports = function setMetadataToLayergroup (layergroupMetadata, includeQu layergroupMetadata.addAnalysesMetadata(user, layergroup, analysesResults, includeQuery); layergroupMetadata.addTurboCartoContextMetadata(layergroup, mapConfig.obj(), context); layergroupMetadata.addAggregationContextMetadata(layergroup, mapConfig.obj(), context); + layergroupMetadata.addDateWrappingMetadata (layergroup, mapConfig.obj()); layergroupMetadata.addTileJsonMetadata(layergroup, user, mapConfig); next(); diff --git a/lib/cartodb/utils/date-wrapper.js b/lib/cartodb/utils/date-wrapper.js index dfbf52dd..3c0907f1 100644 --- a/lib/cartodb/utils/date-wrapper.js +++ b/lib/cartodb/utils/date-wrapper.js @@ -8,19 +8,19 @@ const DATE_OIDS = Object.freeze({ /** * Wrap a query transforming all date columns into a unix epoch - * @param {*} originalQuery - * @param {*} fields + * @param {*} originalQuery + * @param {*} fields */ function wrapDates(originalQuery, fields) { return ` - SELECT - ${fields.map(field => _isDateType(field) ? _castColumnToEpoch(field.name) : `${field.name}`).join(',')} - FROM + SELECT + ${fields.map(field => _isDateType(field) ? _castColumnToEpoch(field.name) : `${field.name}`).join(',')} + FROM (${originalQuery}) _cdb_epoch_transformation `; } /** - * @param {object} field + * @param {object} field */ function _isDateType(field) { return DATE_OIDS.hasOwnProperty(field.dataTypeID); @@ -31,9 +31,25 @@ function _isDateType(field) { * @param {string} column - The name of the date column */ function _castColumnToEpoch(columnName) { - return `date_part('epoch', ${columnName}) as "${columnName}"`; + return `date_part('epoch', "${columnName}") as "${columnName}"`; +} + +function wrappedDates(query) { + if (query.match(/\b_cdb_epoch_transformation\b/)) { + const columns = []; + const fieldMatcher = /\bdate_part\('epoch', "([^"]+)"\) as "([^"]+)"/gmi; + let match; + do { + match = fieldMatcher.exec(query); + if (match && match[1] === match[2]) { + columns.push(match[1]); + } + } while (match); + return columns; + } } module.exports = { - wrapDates + wrapDates, + wrappedDates }; \ No newline at end of file diff --git a/lib/cartodb/utils/layergroup-metadata.js b/lib/cartodb/utils/layergroup-metadata.js index 0e5d702a..62a3cd73 100644 --- a/lib/cartodb/utils/layergroup-metadata.js +++ b/lib/cartodb/utils/layergroup-metadata.js @@ -1,3 +1,5 @@ +const dateWrapper = require('./date-wrapper'); + module.exports = class LayergroupMetadata { constructor (resourceLocator) { this.resourceLocator = resourceLocator; @@ -165,4 +167,18 @@ module.exports = class LayergroupMetadata { }); } } + + addDateWrappingMetadata (layergroup, mapConfig) { + if (layergroup.metadata && Array.isArray(layergroup.metadata.layers) && Array.isArray(mapConfig.layers)) { + layergroup.metadata.layers = layergroup.metadata.layers.map(function(layer, layerIndex) { + const mapConfigLayer = mapConfig.layers[layerIndex]; + const wrappedColumns = dateWrapper.wrappedDates(mapConfigLayer.options.sql); + if (wrappedColumns) { + layer.meta.dates_as_numbers = wrappedColumns; + } + return layer; + }); + } + } + }; From 2ab22882d602a2ac9ae4625831a6c1f35a35f4f3 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Tue, 5 Jun 2018 15:39:15 +0200 Subject: [PATCH 18/28] Slight code trimming --- test/acceptance/date-wrapping.spec.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/acceptance/date-wrapping.spec.js b/test/acceptance/date-wrapping.spec.js index cf8d6508..546a4de8 100644 --- a/test/acceptance/date-wrapping.spec.js +++ b/test/acceptance/date-wrapping.spec.js @@ -42,9 +42,8 @@ describe('date-wrapping', () => { it('should return metadata with casted columns', done => { testClient.getLayergroup(function(err, layergroup) { - const expected = ['date']; assert.ifError(err); - assert.deepEqual(layergroup.metadata.layers[0].meta.dates_as_numbers.sort(), expected.sort()); + assert.deepEqual(layergroup.metadata.layers[0].meta.dates_as_numbers, ['date']); testClient.drain(done); }); From 5b637577c8cec269a59eb6855a02044c10d9e846 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Tue, 5 Jun 2018 16:30:27 +0200 Subject: [PATCH 19/28] Fix conflict resolution gone wrong --- test/acceptance/date-wrapping.spec.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/acceptance/date-wrapping.spec.js b/test/acceptance/date-wrapping.spec.js index 6bbef67f..eccd26b1 100644 --- a/test/acceptance/date-wrapping.spec.js +++ b/test/acceptance/date-wrapping.spec.js @@ -44,8 +44,6 @@ describe('date-wrapping', () => { testClient.getLayergroup(function(err, layergroup) { assert.ifError(err); assert.deepEqual(layergroup.metadata.layers[0].meta.dates_as_numbers, ['date']); - testClient.drain(done); - assert.deepEqual(layergroup.metadata.layers[0].meta.dates_as_numbers.sort(), expected.sort()); done(); }); From 84c34361a002cab797fb0a706f81c88cff8b3674 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Mon, 11 Jun 2018 19:30:48 +0200 Subject: [PATCH 20/28] Fix loop limits, add per layer options in test mapconfig Factory --- test/fixtures/test_mapconfigFactory.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/fixtures/test_mapconfigFactory.js b/test/fixtures/test_mapconfigFactory.js index 4c71bbab..ff554be5 100644 --- a/test/fixtures/test_mapconfigFactory.js +++ b/test/fixtures/test_mapconfigFactory.js @@ -10,8 +10,9 @@ function getVectorMapConfig(opts) { function _generateLayers(opts) { const numberOfLayers = opts.numberOfLayers || 1; const layers = []; - for (let index = 0; index <= numberOfLayers; index++) { - layers.push(_generateLayerConfig(opts)); + for (let index = 0; index < numberOfLayers; index++) { + const layerOptions = (opts.layerOptions || {})[index] || {}; + layers.push(_generateLayerConfig(layerOptions)); } return layers; } From 5407df03fad5e88425a82ebd52456d640a06a3a8 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Mon, 11 Jun 2018 19:31:15 +0200 Subject: [PATCH 21/28] Implement missing tests --- test/acceptance/date-wrapping.spec.js | 178 ++++++++++++++++++++++++-- 1 file changed, 167 insertions(+), 11 deletions(-) diff --git a/test/acceptance/date-wrapping.spec.js b/test/acceptance/date-wrapping.spec.js index eccd26b1..a96a9e39 100644 --- a/test/acceptance/date-wrapping.spec.js +++ b/test/acceptance/date-wrapping.spec.js @@ -85,25 +85,181 @@ describe('date-wrapping', () => { }); }); - describe('when a map instantiation has multiple layers', () => { - beforeEach(() => { - const mapConfig = mapConfigFactory.getVectorMapConfig({ numberOfLayers: 2 }); - testClient = new TestClient(mapConfig); - }); + afterEach(done => testClient.drain(done)); + describe('and both layers have the "dates_as_numbers" option enabled', () => { - // TODO: Pending test - it('should return dates as numbers for every layer'); + beforeEach(() => { + const mapConfig = mapConfigFactory.getVectorMapConfig({ + numberOfLayers: 2, + layerOptions: [ + { dates_as_numbers: true }, + { dates_as_numbers: true } + ] + }); + testClient = new TestClient(mapConfig); + }); + + it('should return dates as numbers for every layer', done => { + testClient.getLayergroup(function(err, layergroup) { + assert.ifError(err); + assert.deepEqual(layergroup.metadata.layers[0].meta.dates_as_numbers, ['date']); + assert.deepEqual(layergroup.metadata.layers[1].meta.dates_as_numbers, ['date']); + }); + + testClient.getTile(0, 0, 0, { format: 'mvt' }, (err, res, mvt) => { + const expected0 = [ + { + type: 'Feature', + id: 1, + geometry: { type: 'Point', coordinates: [0, 0] }, + properties: { _cdb_feature_count: 1, cartodb_id: 0, date: 1527810000 } + }, + { + type: 'Feature', + id: 2, + geometry: { type: 'Point', coordinates: [0, 0] }, + properties: { _cdb_feature_count: 1, cartodb_id: 1, date: 1527900000 } + } + ]; + const expected1 = [ + { + type: 'Feature', + id: 1, + geometry: { type: 'Point', coordinates: [0, 0] }, + properties: { _cdb_feature_count: 1, cartodb_id: 0, date: 1527810000 } + }, + { + type: 'Feature', + id: 2, + geometry: { type: 'Point', coordinates: [0, 0] }, + properties: { _cdb_feature_count: 1, cartodb_id: 1, date: 1527900000 } + } + ]; + const actual0 = JSON.parse(mvt.toGeoJSONSync(0)).features; + const actual1 = JSON.parse(mvt.toGeoJSONSync(1)).features; + + assert.deepEqual(actual0, expected0); + assert.deepEqual(actual1, expected1); + done(); + }); + }); }); describe('and only one layers has the "dates_as_numbers" option enabled', () => { - // TODO: Pending test - it('should return dates as numbers only for the layer with the "dates_as_numbers" flag enabled'); + beforeEach(() => { + const mapConfig = mapConfigFactory.getVectorMapConfig({ + numberOfLayers: 2, + layerOptions: [ + { dates_as_numbers: false }, + { dates_as_numbers: true } + ] + }); + testClient = new TestClient(mapConfig); + }); + + it('should return dates as numbers only for the layer with the "dates_as_numbers" flag enabled', done => { + testClient.getLayergroup(function(err, layergroup) { + assert.ifError(err); + assert.deepEqual(layergroup.metadata.layers[0].meta.dates_as_numbers || [], []); + assert.deepEqual(layergroup.metadata.layers[1].meta.dates_as_numbers, ['date']); + }); + + testClient.getTile(0, 0, 0, { format: 'mvt' }, (err, res, mvt) => { + const expected0 = [ + { + type: 'Feature', + id: 1, + geometry: { type: 'Point', coordinates: [0, 0] }, + properties: { _cdb_feature_count: 1, cartodb_id: 0 } + }, + { + type: 'Feature', + id: 2, + geometry: { type: 'Point', coordinates: [0, 0] }, + properties: { _cdb_feature_count: 1, cartodb_id: 1 } + } + ]; + const expected1 = [ + { + type: 'Feature', + id: 1, + geometry: { type: 'Point', coordinates: [0, 0] }, + properties: { _cdb_feature_count: 1, cartodb_id: 0, date: 1527810000 } + }, + { + type: 'Feature', + id: 2, + geometry: { type: 'Point', coordinates: [0, 0] }, + properties: { _cdb_feature_count: 1, cartodb_id: 1, date: 1527900000 } + } + ]; + const actual0 = JSON.parse(mvt.toGeoJSONSync(0)).features; + const actual1 = JSON.parse(mvt.toGeoJSONSync(1)).features; + + assert.deepEqual(actual0, expected0); + assert.deepEqual(actual1, expected1); + done(); + }); + }); }); describe('and none of the layers has the "dates_as_numbers" option enabled', () => { - // TODO: Pending test - it('should return dates as dates for both layers'); + beforeEach(() => { + const mapConfig = mapConfigFactory.getVectorMapConfig({ + numberOfLayers: 2, + layerOptions: [ + { dates_as_numbers: false }, + { dates_as_numbers: false } + ] + }); + testClient = new TestClient(mapConfig); + }); + + it('should return dates as dates for both layers', done => { + testClient.getLayergroup(function(err, layergroup) { + assert.ifError(err); + assert.deepEqual(layergroup.metadata.layers[0].meta.dates_as_numbers || [], []); + assert.deepEqual(layergroup.metadata.layers[1].meta.dates_as_numbers || [], []); + }); + + testClient.getTile(0, 0, 0, { format: 'mvt' }, (err, res, mvt) => { + const expected0 = [ + { + type: 'Feature', + id: 1, + geometry: { type: 'Point', coordinates: [0, 0] }, + properties: { _cdb_feature_count: 1, cartodb_id: 0 } + }, + { + type: 'Feature', + id: 2, + geometry: { type: 'Point', coordinates: [0, 0] }, + properties: { _cdb_feature_count: 1, cartodb_id: 1 } + } + ]; + const expected1 = [ + { + type: 'Feature', + id: 1, + geometry: { type: 'Point', coordinates: [0, 0] }, + properties: { _cdb_feature_count: 1, cartodb_id: 0 } + }, + { + type: 'Feature', + id: 2, + geometry: { type: 'Point', coordinates: [0, 0] }, + properties: { _cdb_feature_count: 1, cartodb_id: 1 } + } + ]; + const actual0 = JSON.parse(mvt.toGeoJSONSync(0)).features; + const actual1 = JSON.parse(mvt.toGeoJSONSync(1)).features; + + assert.deepEqual(actual0, expected0); + assert.deepEqual(actual1, expected1); + done(); + }); + }); }); }); }); \ No newline at end of file From 660c1777e3ff460ddb131eb01f43e5d50dd9e702 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Mon, 11 Jun 2018 19:31:42 +0200 Subject: [PATCH 22/28] Fix per-layer date wrapping --- .../models/mapconfig/adapter/vector-mapconfig-adapter.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js b/lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js index aada7b0f..211c574b 100644 --- a/lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js +++ b/lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js @@ -4,7 +4,7 @@ const dateWrapper = require('../../../utils/date-wrapper'); /** * This middleware wraps the layer query transforming the date fields into numbers because mvt tiles * doesnt support dates as primitive type. - * + * * - This middleware is ONLY activated when the `dates_as_numbers` option is enabled for some layer in the mapConfig. */ class VectorMapConfigAdapter { @@ -28,6 +28,9 @@ class VectorMapConfigAdapter { } _wrapLayer(layer, user) { + if (!layer.options.dates_as_numbers) { + return Promise.resolve(layer); + } const originalQuery = layer.options.sql; return this._getColumns(user, originalQuery) .then(result => { From 251570b638859e6bd9247a7cfc8d4dd578d99596 Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Tue, 12 Jun 2018 12:04:13 +0200 Subject: [PATCH 23/28] Fix tests --- test/acceptance/date-wrapping.spec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/acceptance/date-wrapping.spec.js b/test/acceptance/date-wrapping.spec.js index a96a9e39..9bb8098f 100644 --- a/test/acceptance/date-wrapping.spec.js +++ b/test/acceptance/date-wrapping.spec.js @@ -9,7 +9,7 @@ describe('date-wrapping', () => { describe('when a map instantiation has one single layer', () => { describe('and the layer has the "dates_as_numbers" option enabled', () => { beforeEach(() => { - const mapConfig = mapConfigFactory.getVectorMapConfig({ dates_as_numbers: true }); + const mapConfig = mapConfigFactory.getVectorMapConfig({ layerOptions: [{ dates_as_numbers: true }]}); testClient = new TestClient(mapConfig); }); @@ -53,7 +53,7 @@ describe('date-wrapping', () => { describe('and the layer has the "dates_as_numbers" option disabled', () => { beforeEach(() => { - const mapConfig = mapConfigFactory.getVectorMapConfig({ dates_as_numbers: false }); + const mapConfig = mapConfigFactory.getVectorMapConfig({ layerOptions: [{ dates_as_numbers: false }]}); testClient = new TestClient(mapConfig); }); From 4a52620d8362d47579fd87dfdf223b9fbc215aec Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Tue, 12 Jun 2018 12:05:31 +0200 Subject: [PATCH 24/28] Various fixes This avoids errors when trying to wrap dates or detect wrapped dates in non-mapnik layers --- .../adapter/vector-mapconfig-adapter.js | 2 +- lib/cartodb/utils/date-wrapper.js | 24 ++++++++++--------- lib/cartodb/utils/layergroup-metadata.js | 9 ++++--- 3 files changed, 20 insertions(+), 15 deletions(-) diff --git a/lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js b/lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js index 211c574b..a707b5d3 100644 --- a/lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js +++ b/lib/cartodb/models/mapconfig/adapter/vector-mapconfig-adapter.js @@ -28,7 +28,7 @@ class VectorMapConfigAdapter { } _wrapLayer(layer, user) { - if (!layer.options.dates_as_numbers) { + if (!layer.options.dates_as_numbers || !layer.options.sql) { return Promise.resolve(layer); } const originalQuery = layer.options.sql; diff --git a/lib/cartodb/utils/date-wrapper.js b/lib/cartodb/utils/date-wrapper.js index 3c0907f1..d07d43c1 100644 --- a/lib/cartodb/utils/date-wrapper.js +++ b/lib/cartodb/utils/date-wrapper.js @@ -35,17 +35,19 @@ function _castColumnToEpoch(columnName) { } function wrappedDates(query) { - if (query.match(/\b_cdb_epoch_transformation\b/)) { - const columns = []; - const fieldMatcher = /\bdate_part\('epoch', "([^"]+)"\) as "([^"]+)"/gmi; - let match; - do { - match = fieldMatcher.exec(query); - if (match && match[1] === match[2]) { - columns.push(match[1]); - } - } while (match); - return columns; + if (query) { + if (query.match(/\b_cdb_epoch_transformation\b/)) { + const columns = []; + const fieldMatcher = /\bdate_part\('epoch', "([^"]+)"\) as "([^"]+)"/gmi; + let match; + do { + match = fieldMatcher.exec(query); + if (match && match[1] === match[2]) { + columns.push(match[1]); + } + } while (match); + return columns; + } } } diff --git a/lib/cartodb/utils/layergroup-metadata.js b/lib/cartodb/utils/layergroup-metadata.js index 62a3cd73..65fb8f4a 100644 --- a/lib/cartodb/utils/layergroup-metadata.js +++ b/lib/cartodb/utils/layergroup-metadata.js @@ -172,9 +172,12 @@ module.exports = class LayergroupMetadata { if (layergroup.metadata && Array.isArray(layergroup.metadata.layers) && Array.isArray(mapConfig.layers)) { layergroup.metadata.layers = layergroup.metadata.layers.map(function(layer, layerIndex) { const mapConfigLayer = mapConfig.layers[layerIndex]; - const wrappedColumns = dateWrapper.wrappedDates(mapConfigLayer.options.sql); - if (wrappedColumns) { - layer.meta.dates_as_numbers = wrappedColumns; + const layerOptions = mapConfigLayer.options; + if (layerOptions.dates_as_numbers && layerOptions.sql) { + const wrappedColumns = dateWrapper.wrappedDates(layerOptions.sql); + if (wrappedColumns) { + layer.meta.dates_as_numbers = wrappedColumns; + } } return layer; }); From 0cf6605b8dc2d9d8f887eec65c09631c901f398c Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Tue, 12 Jun 2018 12:59:10 +0200 Subject: [PATCH 25/28] Use more precise function name --- lib/cartodb/utils/date-wrapper.js | 4 ++-- lib/cartodb/utils/layergroup-metadata.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/cartodb/utils/date-wrapper.js b/lib/cartodb/utils/date-wrapper.js index d07d43c1..895cb7c8 100644 --- a/lib/cartodb/utils/date-wrapper.js +++ b/lib/cartodb/utils/date-wrapper.js @@ -34,7 +34,7 @@ function _castColumnToEpoch(columnName) { return `date_part('epoch', "${columnName}") as "${columnName}"`; } -function wrappedDates(query) { +function getColumnsWithWrappedDates(query) { if (query) { if (query.match(/\b_cdb_epoch_transformation\b/)) { const columns = []; @@ -53,5 +53,5 @@ function wrappedDates(query) { module.exports = { wrapDates, - wrappedDates + getColumnsWithWrappedDates }; \ No newline at end of file diff --git a/lib/cartodb/utils/layergroup-metadata.js b/lib/cartodb/utils/layergroup-metadata.js index 65fb8f4a..41c3c9af 100644 --- a/lib/cartodb/utils/layergroup-metadata.js +++ b/lib/cartodb/utils/layergroup-metadata.js @@ -174,7 +174,7 @@ module.exports = class LayergroupMetadata { const mapConfigLayer = mapConfig.layers[layerIndex]; const layerOptions = mapConfigLayer.options; if (layerOptions.dates_as_numbers && layerOptions.sql) { - const wrappedColumns = dateWrapper.wrappedDates(layerOptions.sql); + const wrappedColumns = dateWrapper.getColumnsWithWrappedDates(layerOptions.sql); if (wrappedColumns) { layer.meta.dates_as_numbers = wrappedColumns; } From ae7e7578dbb3e9268a35fc92f2f814fa304a0389 Mon Sep 17 00:00:00 2001 From: IagoLast Date: Wed, 13 Jun 2018 09:38:24 +0200 Subject: [PATCH 26/28] Use early return --- lib/cartodb/utils/date-wrapper.js | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/lib/cartodb/utils/date-wrapper.js b/lib/cartodb/utils/date-wrapper.js index 895cb7c8..60e88001 100644 --- a/lib/cartodb/utils/date-wrapper.js +++ b/lib/cartodb/utils/date-wrapper.js @@ -35,19 +35,20 @@ function _castColumnToEpoch(columnName) { } function getColumnsWithWrappedDates(query) { - if (query) { - if (query.match(/\b_cdb_epoch_transformation\b/)) { - const columns = []; - const fieldMatcher = /\bdate_part\('epoch', "([^"]+)"\) as "([^"]+)"/gmi; - let match; - do { - match = fieldMatcher.exec(query); - if (match && match[1] === match[2]) { - columns.push(match[1]); - } - } while (match); - return columns; - } + if(!query){ + return; + } + if (query.match(/\b_cdb_epoch_transformation\b/)) { + const columns = []; + const fieldMatcher = /\bdate_part\('epoch', "([^"]+)"\) as "([^"]+)"/gmi; + let match; + do { + match = fieldMatcher.exec(query); + if (match && match[1] === match[2]) { + columns.push(match[1]); + } + } while (match); + return columns; } } From 649297df8341e77d738b1b95599f3c3f9d6ba231 Mon Sep 17 00:00:00 2001 From: IagoLast Date: Wed, 13 Jun 2018 09:42:53 +0200 Subject: [PATCH 27/28] Use early return --- lib/cartodb/utils/date-wrapper.js | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/lib/cartodb/utils/date-wrapper.js b/lib/cartodb/utils/date-wrapper.js index 60e88001..9d5657b1 100644 --- a/lib/cartodb/utils/date-wrapper.js +++ b/lib/cartodb/utils/date-wrapper.js @@ -38,21 +38,22 @@ function getColumnsWithWrappedDates(query) { if(!query){ return; } - if (query.match(/\b_cdb_epoch_transformation\b/)) { - const columns = []; - const fieldMatcher = /\bdate_part\('epoch', "([^"]+)"\) as "([^"]+)"/gmi; - let match; - do { - match = fieldMatcher.exec(query); - if (match && match[1] === match[2]) { - columns.push(match[1]); - } - } while (match); - return columns; + if (!query.match(/\b_cdb_epoch_transformation\b/)) { + return; } + const columns = []; + const fieldMatcher = /\bdate_part\('epoch', "([^"]+)"\) as "([^"]+)"/gmi; + let match; + do { + match = fieldMatcher.exec(query); + if (match && match[1] === match[2]) { + columns.push(match[1]); + } + } while (match); + return columns; } module.exports = { wrapDates, getColumnsWithWrappedDates -}; \ No newline at end of file +}; From fc9ae9ca2052cd6bfbfe2cb0a03745dbc0fff08e Mon Sep 17 00:00:00 2001 From: Javier Goizueta Date: Wed, 13 Jun 2018 12:54:57 +0200 Subject: [PATCH 28/28] Update NEWS --- NEWS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/NEWS.md b/NEWS.md index fb2d8ea2..c3a51254 100644 --- a/NEWS.md +++ b/NEWS.md @@ -33,6 +33,7 @@ New features: - nock: 9.2.6 - strftime: 0.10.0 - Optional instantiation metadata stats (https://github.com/CartoDB/Windshaft-cartodb/pull/952) +- Experimental dates_as_numbers support Bug Fixes: - Validates tile coordinates (z/x/y) from request params to be a valid integer value.