From fbcfc7a5821362e0e903a8b24239ac95a97a34ad Mon Sep 17 00:00:00 2001
From: Javier Goizueta <jgoizueta@gmail.com>
Date: Thu, 20 Sep 2018 21:12:54 +0200
Subject: [PATCH 01/22] WIP: time dimensions for aggregation

---
 .../models/aggregation/aggregation-query.js   |  96 +++++-
 .../models/aggregation/time-dimension.js      | 278 ++++++++++++++++++
 test/acceptance/aggregation.js                |  40 +++
 3 files changed, 412 insertions(+), 2 deletions(-)
 create mode 100644 lib/cartodb/models/aggregation/time-dimension.js

diff --git a/lib/cartodb/models/aggregation/aggregation-query.js b/lib/cartodb/models/aggregation/aggregation-query.js
index a455e02a..84216e91 100644
--- a/lib/cartodb/models/aggregation/aggregation-query.js
+++ b/lib/cartodb/models/aggregation/aggregation-query.js
@@ -1,5 +1,8 @@
+const timeDimension = require('./time-dimension');
+
 const DEFAULT_PLACEMENT = 'point-sample';
 
+
 /**
  * Returns a template function (function that accepts template parameters and returns a string)
  * to generate an aggregation query.
@@ -113,6 +116,95 @@ const aggregateColumnDefs = ctx => {
 
 const aggregateDimensions = ctx => ctx.dimensions || {};
 
+const timeDimensionParameters = definition => {
+    let group_by_count = definition.step || 1;
+    let group_by_units;
+    let group_by_cycle;
+    switch (definition.group_by) {
+        case 'second':
+        case 'minute':
+        case 'hour':
+        case 'day':
+        case 'week':
+        case 'month':
+        case 'quarter':
+        case 'year':
+        case 'century':
+        case 'millenium':
+            group_by_units = definition.group_by;
+            break;
+        case 'semester':
+            group_by_units = 'month';
+            group_by_count *= 6;
+            break;
+        case 'trimester':
+            group_by_units = 'month';
+            group_by_count *= 4;
+            break;
+        case 'minuteOfHour':
+            group_by_units = 'minute';
+            group_by_cycle = 'hour';
+            break;
+        case 'hourOfDay':
+            group_by_units = 'hour';
+            group_by_cycle = 'day';
+            break;
+        case 'dayOfWeek':
+            group_by_units = 'day';
+            group_by_cycle = 'week';
+            break;
+        case 'dayOfMonth':
+            group_by_units = 'day';
+            group_by_cycle = 'month';
+            break;
+        case 'dayOfYear':
+            group_by_units = 'day';
+            group_by_cycle = 'year';
+            break;
+        case 'weekOfYear':
+            group_by_units = 'week';
+            group_by_cycle = 'year';
+            break;
+        case 'monthOfYear':
+            group_by_units = 'month';
+            group_by_cycle = 'year';
+            break;
+        case 'quarterOfYear':
+            group_by_units = 'quarter';
+            group_by_cycle = 'year';
+            break;
+        case 'trimesterOfYear':
+            group_by_units = 'month';
+            group_by_count *= 4;
+            group_by_cycle = 'year';
+            break;
+        case 'semesterOfYear':
+            group_by_units = 'month';
+            group_by_count *= 6;
+            group_by_cycle = 'year';
+            break;
+        default:
+            throw new Error(`Invalid time grouping ${definition.group_by}`);
+    }
+    // definition.column should correspond to a wrapped date column
+    return {
+        time: `to_timestamp("${definition.column}")`,
+        timezone: definition.timezone || 'utc',
+        granularity: group_by_units,
+        multiplicity: group_by_count || 1,
+        cycle: group_by_cycle,
+        offset: definition.offset || 0
+    };
+};
+
+const dimensionExpression = definition => {
+    if (typeof(definition) === 'string') {
+        return `"definition"`;
+    }
+    // Currently only time dimensions are supported with parameters
+    return timeDimension(timeDimensionParameters(definition));
+};
+
 const dimensionNames = (ctx, table) => {
     let dimensions = aggregateDimensions(ctx);
     if (table) {
@@ -128,8 +220,8 @@ const dimensionNames = (ctx, table) => {
 const dimensionDefs = ctx => {
     let dimensions = aggregateDimensions(ctx);
     return sep(Object.keys(dimensions).map(dimension_name => {
-        const expression = dimensions[dimension_name];
-        return `"${expression}" AS "${dimension_name}"`;
+        const expression = dimensionExpression(dimensions[dimension_name]);
+        return `${expression} AS "${dimension_name}"`;
     }));
 };
 
diff --git a/lib/cartodb/models/aggregation/time-dimension.js b/lib/cartodb/models/aggregation/time-dimension.js
new file mode 100644
index 00000000..16eb468c
--- /dev/null
+++ b/lib/cartodb/models/aggregation/time-dimension.js
@@ -0,0 +1,278 @@
+const MONTH_SECONDS = 365.2425 / 12 * 24 * 3600 // PG intervals use 30 * 24 * 3600
+const YEAR_SECONDS  = 12 * MONTH_SECONDS;
+
+// time unit durations
+const usecs = {
+    second: 1,
+    minute: 60,
+    hour: 3600,
+    day: 24 * 3600,
+    week: 7 * 24 * 3600,
+    month: MONTH_SECONDS,
+    year:  YEAR_SECONDS,
+
+    quarter: 3 * MONTH_SECONDS,
+    semester: 6 * MONTH_SECONDS,
+    trimester: 4 * MONTH_SECONDS,
+    decade: 12 * YEAR_SECONDS,
+    century: 100 * YEAR_SECONDS,
+    millennium: 1000 * YEAR_SECONDS
+};
+
+serialParts = {
+    second: {
+        sql: `FLOOR(date_part('epoch', $t))`,
+        zeroBased: true
+    },
+    minute: {
+        sql: `date_part('epoch', date_trunc('day', $t))/60`,
+        zeroBased: true
+    },
+    hour: {
+        sql: `date_part('epoch', date_trunc('day', $t))/(60*60)`,
+        zeroBased: true
+    },
+    day: {
+        sql: `date_part('epoch', date_trunc('day', $t))/(24*60*60) + 1`,
+        zeroBased: false
+    },
+    week: {
+        sql: `date_part('epoch', date_trunc('week', $t))/(7*24*60*60) + 1`,
+        zeroBaseed: false
+    },
+    month: {
+        sql: `date_part('month', $t) + 12*(date_part('year', $t)-date_part('year', to_timestamp(0.0)))`,
+        zeroBased: false
+    },
+    quarter: {
+        sql: `date_part('quarter', $t) + 4*(date_part('year', $t)-date_part('year', to_timestamp(0.0)))`,
+        zeroBased: false
+    },
+    year: {
+        sql: `date_part('year', $t)-date_part('year', to_timestamp(0.0))`,
+        zeroBased: false
+    }
+};
+
+function serialSqlExpr(t, tz, u, m = 1, u_offset = 0, m_offset = 0) {
+    [u, m, u_offset] = serialNormalize(u, m, u_offset);
+    let { sql, zeroBased } = serialParts[u];
+    const column = timeExpression(t, tz);
+    const serial = sql.replace(/\$t/g, column);
+    let expr = serial;
+    if (u_offset !== 0) {
+        expr = `expr - ${u_offset}`;
+    }
+    if (m !== 1) {
+        if (zeroBased) {
+            expr = `FLOOR((${expr})/(${m}::double precision))::int`;
+        } else {
+            expr = `CEIL((${expr})/(${m}::double precision))::int`;
+        }
+    } else {
+        expr = `ROUND(${expr})::int`;
+    }
+    if (m_offset !== 0) {
+        expr = `(${expr} - 1)`;
+    }
+    return expr;
+}
+
+function serialNormalize(u, m, u_offset) {
+    if (u === 'semester') {
+        u = 'month';
+        m *= 6;
+        u_offset *= 6;
+    } else if (u === 'trimester') {
+        u = 'month';
+        m *= 4;
+        u_offset *= 4;
+    } else if (u === 'decade') {
+        u = 'year';
+        m *= 10;
+        u_offset *= 10
+    } else if (u === 'century') {
+        u = 'year';
+        m *= 100;
+        u_offset *= 100
+    } else if (u === 'millenium') {
+        u = 'year';
+        m *= 1000;
+        u_offset *= 1000
+    }
+    return [u, m, u_offset];
+}
+
+function cyclicNormalize(u, m, c, c_offset) {
+    if (u === 'month' && m === 3) {
+        u = 'quarter';
+        m = 1;
+    } else if (u === 'month' && m === 6) {
+        u = 'semester';
+        m = 1;
+    } else if (u === 'month' && m === 4) {
+        u = 'trimester';
+        m = 1;
+    }
+    if (m !== 1) {
+        throw new Error(`invalid multiplicity ${m} for cyclic ${u}`);
+    }
+    return [u, m, c, c_offset];
+}
+
+// timezones can be defined either by an numeric offset in seconds or by
+// a valid (case-insensitive) tz/PG name;
+// they include abbreviations defined by PG (which have precedence and
+// are fixed offsets, not handling DST) or general names that can handle DST.
+function timezone(tz) {
+    if (isFinite(tz)) {
+        return `INTERVAL '${tz} seconds'`;
+    }
+    return `'${tz}'`
+}
+
+// We assume t is a TIMESTAMP WITH TIME ZONE.
+// If this was to be used with a t which is a TIMESTAMP or TIME (no time zone)
+// it should be converted with `timezone('utc',t)` to a type with time zone.
+// Note that by default CARTO uses timestamp with time zone columns for dates
+// and VectorMapConfigAdapter converts them to epoch numbers.
+// So, for using this with aggregations, relying on dates & times
+// converted to UTC UNIX epoch numbers, apply `to_timestamp` to the
+// (converted) column.
+function timeExpression(t, tz) {
+   if (tz !== undefined) {
+        return `timezone(${timezone(tz)}, ${t})`
+   }
+   return t;
+}
+
+function cyclicSqlExpr(t, tz, u, c, c_offset = 0, m = 1) {
+    [u, m, c, c_offset] = cyclicNormalize(u, m, c, c_offset);
+    const comb = `${u}/${c}`;
+    const column = timeExpression(t, tz);
+    let expr;
+
+    if (m === 1) {
+        switch (comb) {
+            case 'day/week':
+                // result: 0-6
+                // c_offset = 0 => 0 = sunday; 1 => 0 = monday...
+                // let expr = `EXTRACT(DOW FROM ${column})`;
+                expr = `date_part('dow', ${column})`;
+                if (c_offset !== 0) {
+                    expr = `(${expr} - ${c_offset}) % 7`;
+                }
+                return expr;
+
+                // iso dow monday=1, no offset:
+                //   `EXTRACT(ISODOW FROM ${column})`
+                // iso dow 1-6, offset 0 => 1 = mondayº
+                //   expr = `date_part('dow, ${column})`;
+                //   c_offset += 1;
+                //   expr = `(${expr} - ${c_offset}) % 7 + 1`;
+
+            case 'day/month':
+                // result: 1-31
+                // c_offset not supported
+                return `date_part('day', ${column})`;
+
+            case 'day/year':
+                // result: 1-366
+                // c_offset not supported
+                return `date_part('doy', ${column})`;
+
+            case 'hour/day':
+                // result: 0-23
+                expr = `date_part('hour', ${column})`;
+                if (c_offset !== 0) {
+                    expr = `(${expr} - ${c_offset}) % 24`;
+                }
+                return expr;
+
+            case 'month/year':
+                // result 1-12
+                expr = `date_part('month', ${column})`;
+                if (c_offset !== 0) {
+                    expr = `((${expr} - ${c_offset} - 1) % 12) + 1`;
+                }
+                return expr;
+
+            case 'quarter/year':
+                // result 1-4
+                expr = `date_part('quarter', ${column})`;
+                if (c_offset !== 0) {
+                    expr = `((${expr} - ${c_offset} - 1) % 4) + 1`;
+                }
+                return expr;
+
+            case 'semester/year':
+                // result 1-2
+                expr = `FLOOR((date_part('month', ${column})-1)/6.0) + 1`;
+                if (c_offset !== 0) {
+                    expr = `((${expr} - ${c_offset} - 1) % 2) + 1`;
+                }
+                return expr;
+
+            case 'trimester/year':
+                // result 1-3
+                expr = `FLOOR((date_part('month', ${column})-1)/4.0) + 1`;
+                if (c_offset !== 0) {
+                    expr = `((${expr} - ${c_offset} - 1) % 3) + 1`;
+                }
+                return expr;
+
+            case 'week/year':
+                // result 1-52
+                expr = `date_part('week', ${column})`;
+                if (c_offset !== 0) {
+                    expr = `((${expr} - ${c_offset} - 1) % 52) + 1`;
+                }
+                return expr;
+
+            case 'minute/hour':
+                // result 0-59
+                expr = `date_part('minute', ${column})`;
+                if (c_offset !== 0) {
+                    expr = `((${expr} - ${c_offset}) % 60)`;
+                }
+                return expr;
+        }
+    }
+    return genericCyclicSqlExpr(t, u, c, c_offset, m);
+}
+
+function genericCyclicSqlExpr(t, tz, u, c, c_offset = 0, m = 1) {
+    const usec = usecs[u];
+    const csec = usecs[c];
+    const column = timeExpression(t, tz);
+    return `((FLOOR(date_part('epoch', ${column})/(${usec*m}))*(${usec*m})+${c_offset}) % ${csec})/${usec*m}`;
+}
+
+function validateParameters(params) {
+    return true;
+}
+
+function classificationSql(params) {
+    validateParameters(params);
+    if (params.cycle) {
+        return cyclicSqlExpr(
+            params.time,
+            params.timezone || 'utc',
+            params.granularity,
+            params.cycle,
+            params.offset || 0,
+            params.multiplicity || 1
+        );
+    } else {
+        return serialSqlExpr(
+            params.time,
+            params.timezone || 'utc',
+            params.granularity,
+            params.multiplicity || 1,
+            params.offset || 0,
+            0
+        );
+
+    }
+}
+module.exports = classificationSql;
diff --git a/test/acceptance/aggregation.js b/test/acceptance/aggregation.js
index 39fb7845..427983e4 100644
--- a/test/acceptance/aggregation.js
+++ b/test/acceptance/aggregation.js
@@ -878,6 +878,46 @@ describe('aggregation', function () {
                 });
             });
 
+            it('time dimensions',
+            function (done) {
+                this.mapConfig = createVectorMapConfig([
+                    {
+                        type: 'cartodb',
+                        options: {
+                            sql: POINTS_SQL_TIMESTAMP_1,
+                            dates_as_numbers: true,
+                            aggregation: {
+                                threshold: 1,
+                                dimensions: {
+                                    dow: {
+                                        column: 'date',
+                                        group_by: 'dayOfWeek'
+                                    }
+                                }
+                            }
+                        }
+                    }
+                ]);
+
+                this.testClient = new TestClient(this.mapConfig);
+                const options = {
+                    format: 'mvt'
+                };
+                this.testClient.getTile(0, 0, 0, options, (err, res, tile) => {
+                    if (err) {
+                        return done(err);
+                    }
+
+                    const tileJSON = tile.toJSON();
+
+                    tileJSON[0].features.forEach(feature => assert.equal(typeof feature.properties.dow, 'number'));
+
+                    done();
+                });
+            });
+
+
+
             ['centroid', 'point-sample', 'point-grid'].forEach(placement => {
                 it(`dimensions should work for ${placement} placement`, function(done) {
                     this.mapConfig = createVectorMapConfig([

From fbf3fd9d8c8499d735887bc76d2f0330394b2c59 Mon Sep 17 00:00:00 2001
From: Javier Goizueta <jgoizueta@gmail.com>
Date: Tue, 25 Sep 2018 19:10:56 +0200
Subject: [PATCH 02/22] Support old and new dimension definitions

---
 .../models/aggregation/aggregation-query.js   | 33 +++++++++++--------
 1 file changed, 19 insertions(+), 14 deletions(-)

diff --git a/lib/cartodb/models/aggregation/aggregation-query.js b/lib/cartodb/models/aggregation/aggregation-query.js
index 84216e91..7cdbe265 100644
--- a/lib/cartodb/models/aggregation/aggregation-query.js
+++ b/lib/cartodb/models/aggregation/aggregation-query.js
@@ -197,31 +197,36 @@ const timeDimensionParameters = definition => {
     };
 };
 
-const dimensionExpression = definition => {
+// Adapt old-style dimension definitions for backwards compatibility
+const adaptDimensionDefinition = definition => {
     if (typeof(definition) === 'string') {
-        return `"definition"`;
+        return { column: definition }
+    }
+    return definition;
+};
+
+const dimensionExpression = definition => {
+    if (definition.group_by) {
+        // Currently only time dimensions are supported with parameters
+        return timeDimension(timeDimensionParameters(definition));
+    } else {
+        return `"${definition.column}"`;
     }
-    // Currently only time dimensions are supported with parameters
-    return timeDimension(timeDimensionParameters(definition));
 };
 
 const dimensionNames = (ctx, table) => {
     let dimensions = aggregateDimensions(ctx);
-    if (table) {
-        return sep(Object.keys(dimensions).map(
-            dimension_name => `${table}."${dimension_name}"`
-        ));
-    }
-    return sep(Object.keys(dimensions).map(dimension_name => {
-        return `"${dimension_name}"`;
+    return sep(Object.keys(dimensions).map(dimensionName => {
+        return table ? `${table}."${dimensionName}"` : `"${dimensionName}"`;
     }));
 };
 
 const dimensionDefs = ctx => {
     let dimensions = aggregateDimensions(ctx);
-    return sep(Object.keys(dimensions).map(dimension_name => {
-        const expression = dimensionExpression(dimensions[dimension_name]);
-        return `${expression} AS "${dimension_name}"`;
+    return sep(Object.keys(dimensions).map(dimensionName => {
+        const dimension = adaptDimensionDefinition(dimensions[dimensionName]);
+        const expression = dimensionExpression(dimension);
+        return `${expression} AS "${dimensionName}"`;
     }));
 };
 

From dede22c91596041417b13746f3b1f416cdbbdea9 Mon Sep 17 00:00:00 2001
From: Javier Goizueta <jgoizueta@gmail.com>
Date: Wed, 3 Oct 2018 17:05:58 +0200
Subject: [PATCH 03/22] Changes in time dimensions API

Use single `starting` epoch instead of various offsets.
Add ISO text representation.
Adopt ISO conventions for day of week and week of year.
Rename internal parameters for consistency with external API.
---
 .../models/aggregation/aggregation-query.js   |  16 +-
 .../models/aggregation/time-dimension.js      | 155 +++++++++++-------
 2 files changed, 108 insertions(+), 63 deletions(-)

diff --git a/lib/cartodb/models/aggregation/aggregation-query.js b/lib/cartodb/models/aggregation/aggregation-query.js
index 7cdbe265..3e6ba873 100644
--- a/lib/cartodb/models/aggregation/aggregation-query.js
+++ b/lib/cartodb/models/aggregation/aggregation-query.js
@@ -41,13 +41,15 @@ const templateForOptions = (options) => {
  * When placement, columns or dimensions are specified, columns are aggregated as requested
  * (by default only _cdb_feature_count) and with the_geom_webmercator as defined by placement.
  */
-const queryForOptions = (options) => templateForOptions(options)({
+const queryForOptions = (options) => {
+    return templateForOptions(options)({
     sourceQuery: options.query,
     res: 256/options.resolution,
     columns: options.columns,
     dimensions: options.dimensions,
     filters: options.filters
-});
+    });
+};
 
 module.exports = queryForOptions;
 
@@ -190,17 +192,17 @@ const timeDimensionParameters = definition => {
     return {
         time: `to_timestamp("${definition.column}")`,
         timezone: definition.timezone || 'utc',
-        granularity: group_by_units,
-        multiplicity: group_by_count || 1,
-        cycle: group_by_cycle,
-        offset: definition.offset || 0
+        group_by_units,
+        group_by_count,
+        group_by_cycle,
+        epoch: definition.starting
     };
 };
 
 // Adapt old-style dimension definitions for backwards compatibility
 const adaptDimensionDefinition = definition => {
     if (typeof(definition) === 'string') {
-        return { column: definition }
+        return { column: definition };
     }
     return definition;
 };
diff --git a/lib/cartodb/models/aggregation/time-dimension.js b/lib/cartodb/models/aggregation/time-dimension.js
index 16eb468c..476fc4ca 100644
--- a/lib/cartodb/models/aggregation/time-dimension.js
+++ b/lib/cartodb/models/aggregation/time-dimension.js
@@ -1,4 +1,4 @@
-const MONTH_SECONDS = 365.2425 / 12 * 24 * 3600 // PG intervals use 30 * 24 * 3600
+const MONTH_SECONDS = 365.2425 / 12 * 24 * 3600; // PG intervals use 30 * 24 * 3600
 const YEAR_SECONDS  = 12 * MONTH_SECONDS;
 
 // time unit durations
@@ -19,50 +19,54 @@ const usecs = {
     millennium: 1000 * YEAR_SECONDS
 };
 
-serialParts = {
+const YEARSPAN = "(date_part('year', $t)-date_part('year', $epoch))";
+// Note that SECONDSPAN is not a UTC epoch, but an epoch in the specified TZ,
+// so we can use it to compute any multiple of seconds with it  without using date_part or date_trunc
+const SECONDSPAN = "(date_part('epoch', $t) - date_part('epoch', $epoch))";
+
+const serialParts = {
     second: {
-        sql: `FLOOR(date_part('epoch', $t))`,
+        sql: `FLOOR(${SECONDSPAN})`,
         zeroBased: true
     },
     minute: {
-        sql: `date_part('epoch', date_trunc('day', $t))/60`,
+        sql: `FLOOR(${SECONDSPAN}/60)`,
         zeroBased: true
     },
     hour: {
-        sql: `date_part('epoch', date_trunc('day', $t))/(60*60)`,
+        sql: `FLOOR(${SECONDSPAN}/3600)`,
         zeroBased: true
     },
     day: {
-        sql: `date_part('epoch', date_trunc('day', $t))/(24*60*60) + 1`,
+        sql: `1 + FLOOR(${SECONDSPAN}/86400)`,
         zeroBased: false
     },
     week: {
-        sql: `date_part('epoch', date_trunc('week', $t))/(7*24*60*60) + 1`,
-        zeroBaseed: false
+        sql: `1 + FLOOR(${SECONDSPAN}/(7*86400))`,
+        zeroBased: false
     },
     month: {
-        sql: `date_part('month', $t) + 12*(date_part('year', $t)-date_part('year', to_timestamp(0.0)))`,
+        sql: `1 + date_part('month', $t) - date_part('month', $epoch) + 12*${YEARSPAN}`,
         zeroBased: false
     },
     quarter: {
-        sql: `date_part('quarter', $t) + 4*(date_part('year', $t)-date_part('year', to_timestamp(0.0)))`,
+        sql: `1 + date_part('quarter', $t) - date_part('quarter', $epoch) + 4*${YEARSPAN}`,
         zeroBased: false
     },
     year: {
-        sql: `date_part('year', $t)-date_part('year', to_timestamp(0.0))`,
+        // TODO: isn't more meaningful to ignore the epoch here and return date_part('year', $t)
+        sql: `1 + ${YEARSPAN}`,
         zeroBased: false
     }
 };
 
-function serialSqlExpr(t, tz, u, m = 1, u_offset = 0, m_offset = 0) {
-    [u, m, u_offset] = serialNormalize(u, m, u_offset);
+function serialSqlExpr(t, tz, u, m = 1, starting = undefined) {
+    [u, m] = serialNormalize(u, m);
     let { sql, zeroBased } = serialParts[u];
     const column = timeExpression(t, tz);
-    const serial = sql.replace(/\$t/g, column);
+    const epoch  = epochExpression(starting);
+    const serial = sql.replace(/\$t/g, column).replace(/\$epoch/g, epoch);
     let expr = serial;
-    if (u_offset !== 0) {
-        expr = `expr - ${u_offset}`;
-    }
     if (m !== 1) {
         if (zeroBased) {
             expr = `FLOOR((${expr})/(${m}::double precision))::int`;
@@ -70,37 +74,54 @@ function serialSqlExpr(t, tz, u, m = 1, u_offset = 0, m_offset = 0) {
             expr = `CEIL((${expr})/(${m}::double precision))::int`;
         }
     } else {
-        expr = `ROUND(${expr})::int`;
-    }
-    if (m_offset !== 0) {
-        expr = `(${expr} - 1)`;
+        expr = `(${expr})::int`;
     }
     return expr;
 }
 
-function serialNormalize(u, m, u_offset) {
+const isoParts = {
+    second: `to_char($t, 'YYYY-MM-DD"T"HH:MI:SS')`,
+    minute: `to_char($t, 'YYYY-MM-DD"T"HH:MI')`,
+    hour: `to_char($t, 'YYYY-MM-DD"T"HH')`,
+    day: `to_char($t, 'YYYY-MM-DD')`,
+    month: `to_char($t, 'YYYY-MM')`,
+    year: `to_char($t, 'YYYY')`,
+    week: `to_char($t, 'IYYY-"W"IW')`,
+    quarter: `to_char($t, 'YYYY-"Q"Q')`,
+    semester: `to_char($t, 'YYYY"S"') || to_char(CEIL(date_part('month', $t)/6), '9')`,
+    trimester: `to_char($t, 'YYYY"t"') || to_char(CEIL(date_part('month', $t)/4), '9')`,
+    decade: `to_char(date_part('decade', $t), '"D"999')`,
+    century: `to_char($t, '"C"CC')`,
+    millennium: `to_char(date_part('millenium', $t), '"M"999')`
+};
+
+function isoSqlExpr(t, tz, u, m = 1) {
+    const column = timeExpression(t, tz);
+    if (m > 1) {
+        // TODO: it would be sensible to return the ISO of the firt unit in the period
+        throw new Error('Multiple time units not supported for ISO format');
+    }
+    return isoParts[u].replace(/\$t/g, column);
+}
+
+function serialNormalize(u, m) {
     if (u === 'semester') {
         u = 'month';
         m *= 6;
-        u_offset *= 6;
     } else if (u === 'trimester') {
         u = 'month';
         m *= 4;
-        u_offset *= 4;
     } else if (u === 'decade') {
         u = 'year';
         m *= 10;
-        u_offset *= 10
     } else if (u === 'century') {
         u = 'year';
         m *= 100;
-        u_offset *= 100
     } else if (u === 'millenium') {
         u = 'year';
         m *= 1000;
-        u_offset *= 1000
     }
-    return [u, m, u_offset];
+    return [u, m];
 }
 
 function cyclicNormalize(u, m, c, c_offset) {
@@ -128,7 +149,7 @@ function timezone(tz) {
     if (isFinite(tz)) {
         return `INTERVAL '${tz} seconds'`;
     }
-    return `'${tz}'`
+    return `'${tz}'`;
 }
 
 // We assume t is a TIMESTAMP WITH TIME ZONE.
@@ -141,35 +162,48 @@ function timezone(tz) {
 // (converted) column.
 function timeExpression(t, tz) {
    if (tz !== undefined) {
-        return `timezone(${timezone(tz)}, ${t})`
+        return `timezone(${timezone(tz)}, ${t})`;
    }
    return t;
 }
 
+// Epoch should be an ISO timestamp literal without time zone
+// (it is interpreted as in the defined timzezone for the input time)
+// It can be partial, e.g. 'YYYY', 'YYYY-MM', 'YYYY-MM-DDTHH', etc.
+// Defaults are applied: YYYY=0001, MM=01, DD=01, HH=00, MM=00, S=00
+// It returns a timestamp without time zone
+function epochExpression(epoch) {
+    const format = /^(\d\d\d\d)(?:\-?(\d\d)(?:\-?(\d\d)(?:[T\s]?(\d\d)(?:(\d\d)(?:\:(\d\d))?)?)?)?)?$/;
+    const match = epoch.match(format) || [];
+    const year = match[1] || '0001';
+    const month = match[2] || '01';
+    const day = match[3] || '01';
+    const hour = match[4] || '00';
+    const minute = match[5] || '00';
+    const second = match[6] || '00';
+    epoch = `${year}-${month}-${day}T${hour}:${minute}:${second}`;
+    return `TIMESTAMP '${epoch}'`;
+ }
+
 function cyclicSqlExpr(t, tz, u, c, c_offset = 0, m = 1) {
     [u, m, c, c_offset] = cyclicNormalize(u, m, c, c_offset);
     const comb = `${u}/${c}`;
     const column = timeExpression(t, tz);
     let expr;
 
+    // TODO: drop offset support
+
     if (m === 1) {
         switch (comb) {
             case 'day/week':
-                // result: 0-6
-                // c_offset = 0 => 0 = sunday; 1 => 0 = monday...
-                // let expr = `EXTRACT(DOW FROM ${column})`;
-                expr = `date_part('dow', ${column})`;
-                if (c_offset !== 0) {
-                    expr = `(${expr} - ${c_offset}) % 7`;
+                if (c_offset === 0) {
+                    // 1 = monday; 7 = sunday;
+                    return `date_part('isodow', ${column})`;
                 }
-                return expr;
-
-                // iso dow monday=1, no offset:
-                //   `EXTRACT(ISODOW FROM ${column})`
-                // iso dow 1-6, offset 0 => 1 = mondayº
-                //   expr = `date_part('dow, ${column})`;
-                //   c_offset += 1;
-                //   expr = `(${expr} - ${c_offset}) % 7 + 1`;
+                // iso dow 1-6, offset 0 => 1 = monday
+                expr = `date_part('dow, ${column})`;
+                c_offset += 1;
+                return `(${expr} - ${c_offset}) % 7 + 1`;
 
             case 'day/month':
                 // result: 1-31
@@ -222,10 +256,10 @@ function cyclicSqlExpr(t, tz, u, c, c_offset = 0, m = 1) {
                 return expr;
 
             case 'week/year':
-                // result 1-52
+                // result 1-53
                 expr = `date_part('week', ${column})`;
                 if (c_offset !== 0) {
-                    expr = `((${expr} - ${c_offset} - 1) % 52) + 1`;
+                    expr = `((${expr} - ${c_offset} - 1) % 53) + 1`;
                 }
                 return expr;
 
@@ -238,6 +272,7 @@ function cyclicSqlExpr(t, tz, u, c, c_offset = 0, m = 1) {
                 return expr;
         }
     }
+    // TODO: remove generic expression, reaching here should error
     return genericCyclicSqlExpr(t, u, c, c_offset, m);
 }
 
@@ -248,29 +283,37 @@ function genericCyclicSqlExpr(t, tz, u, c, c_offset = 0, m = 1) {
     return `((FLOOR(date_part('epoch', ${column})/(${usec*m}))*(${usec*m})+${c_offset}) % ${csec})/${usec*m}`;
 }
 
-function validateParameters(params) {
+function validateParameters(_params) {
     return true;
 }
 
 function classificationSql(params) {
     validateParameters(params);
-    if (params.cycle) {
+    if (params.group_by_cycle) {
+        // TODO: validate group_by_count === 1, No epoch
         return cyclicSqlExpr(
             params.time,
             params.timezone || 'utc',
-            params.granularity,
-            params.cycle,
-            params.offset || 0,
-            params.multiplicity || 1
+            params.group_by_units,
+            params.group_by_cycle,
+            0,
+            params.group_by_count || 1
+        );
+    } else if (params.format === 'iso') {
+        // TODO: validate group_by_count === 1, No epoch
+        return isoSqlExpr(
+            params.time,
+            params.timezone || 'utc',
+            params.group_by_units,
+            params.group_by_count || 1
         );
     } else {
         return serialSqlExpr(
             params.time,
             params.timezone || 'utc',
-            params.granularity,
-            params.multiplicity || 1,
-            params.offset || 0,
-            0
+            params.group_by_units,
+            params.group_by_count || 1,
+            params.epoch
         );
 
     }

From a7d5415f64683a8b606ad40dd659e62967a93293 Mon Sep 17 00:00:00 2001
From: Javier Goizueta <jgoizueta@gmail.com>
Date: Wed, 3 Oct 2018 17:12:01 +0200
Subject: [PATCH 04/22] Remove offsets from time dimension computations

---
 .../models/aggregation/time-dimension.js      | 67 ++++---------------
 1 file changed, 14 insertions(+), 53 deletions(-)

diff --git a/lib/cartodb/models/aggregation/time-dimension.js b/lib/cartodb/models/aggregation/time-dimension.js
index 476fc4ca..8d39b671 100644
--- a/lib/cartodb/models/aggregation/time-dimension.js
+++ b/lib/cartodb/models/aggregation/time-dimension.js
@@ -124,7 +124,7 @@ function serialNormalize(u, m) {
     return [u, m];
 }
 
-function cyclicNormalize(u, m, c, c_offset) {
+function cyclicNormalize(u, m, c) {
     if (u === 'month' && m === 3) {
         u = 'quarter';
         m = 1;
@@ -138,7 +138,7 @@ function cyclicNormalize(u, m, c, c_offset) {
     if (m !== 1) {
         throw new Error(`invalid multiplicity ${m} for cyclic ${u}`);
     }
-    return [u, m, c, c_offset];
+    return [u, m, c];
 }
 
 // timezones can be defined either by an numeric offset in seconds or by
@@ -185,95 +185,56 @@ function epochExpression(epoch) {
     return `TIMESTAMP '${epoch}'`;
  }
 
-function cyclicSqlExpr(t, tz, u, c, c_offset = 0, m = 1) {
-    [u, m, c, c_offset] = cyclicNormalize(u, m, c, c_offset);
+function cyclicSqlExpr(t, tz, u, c, m = 1) {
+    [u, m, c] = cyclicNormalize(u, m, c);
     const comb = `${u}/${c}`;
     const column = timeExpression(t, tz);
-    let expr;
-
-    // TODO: drop offset support
 
     if (m === 1) {
         switch (comb) {
             case 'day/week':
-                if (c_offset === 0) {
-                    // 1 = monday; 7 = sunday;
-                    return `date_part('isodow', ${column})`;
-                }
-                // iso dow 1-6, offset 0 => 1 = monday
-                expr = `date_part('dow, ${column})`;
-                c_offset += 1;
-                return `(${expr} - ${c_offset}) % 7 + 1`;
+                // 1 = monday; 7 = sunday;
+                return `date_part('isodow', ${column})`;
 
             case 'day/month':
                 // result: 1-31
-                // c_offset not supported
                 return `date_part('day', ${column})`;
 
             case 'day/year':
                 // result: 1-366
-                // c_offset not supported
                 return `date_part('doy', ${column})`;
 
             case 'hour/day':
                 // result: 0-23
-                expr = `date_part('hour', ${column})`;
-                if (c_offset !== 0) {
-                    expr = `(${expr} - ${c_offset}) % 24`;
-                }
-                return expr;
+                return `date_part('hour', ${column})`;
 
             case 'month/year':
                 // result 1-12
-                expr = `date_part('month', ${column})`;
-                if (c_offset !== 0) {
-                    expr = `((${expr} - ${c_offset} - 1) % 12) + 1`;
-                }
-                return expr;
+                return `date_part('month', ${column})`;
 
             case 'quarter/year':
                 // result 1-4
-                expr = `date_part('quarter', ${column})`;
-                if (c_offset !== 0) {
-                    expr = `((${expr} - ${c_offset} - 1) % 4) + 1`;
-                }
-                return expr;
+                return `date_part('quarter', ${column})`;
 
             case 'semester/year':
                 // result 1-2
-                expr = `FLOOR((date_part('month', ${column})-1)/6.0) + 1`;
-                if (c_offset !== 0) {
-                    expr = `((${expr} - ${c_offset} - 1) % 2) + 1`;
-                }
-                return expr;
+                return `FLOOR((date_part('month', ${column})-1)/6.0) + 1`;
 
             case 'trimester/year':
                 // result 1-3
-                expr = `FLOOR((date_part('month', ${column})-1)/4.0) + 1`;
-                if (c_offset !== 0) {
-                    expr = `((${expr} - ${c_offset} - 1) % 3) + 1`;
-                }
-                return expr;
+                return `FLOOR((date_part('month', ${column})-1)/4.0) + 1`;
 
             case 'week/year':
                 // result 1-53
-                expr = `date_part('week', ${column})`;
-                if (c_offset !== 0) {
-                    expr = `((${expr} - ${c_offset} - 1) % 53) + 1`;
-                }
-                return expr;
+                return `date_part('week', ${column})`;
 
             case 'minute/hour':
                 // result 0-59
-                expr = `date_part('minute', ${column})`;
-                if (c_offset !== 0) {
-                    expr = `((${expr} - ${c_offset}) % 60)`;
-                }
-                return expr;
+                return `date_part('minute', ${column})`;
         }
     }
     // TODO: remove generic expression, reaching here should error
-    return genericCyclicSqlExpr(t, u, c, c_offset, m);
+    return genericCyclicSqlExpr(t, u, c, 0, m);
 }
 
 function genericCyclicSqlExpr(t, tz, u, c, c_offset = 0, m = 1) {

From 96ba07569871d553f14bc7f8464cacdbad74e7df Mon Sep 17 00:00:00 2001
From: Javier Goizueta <jgoizueta@gmail.com>
Date: Wed, 3 Oct 2018 18:57:00 +0200
Subject: [PATCH 05/22] Unify handling of cyclic time groupings

Remove generic cyclic grouping
---
 .../models/aggregation/aggregation-query.js   | 76 +--------------
 .../models/aggregation/time-dimension.js      | 96 +++++++------------
 2 files changed, 37 insertions(+), 135 deletions(-)

diff --git a/lib/cartodb/models/aggregation/aggregation-query.js b/lib/cartodb/models/aggregation/aggregation-query.js
index 3e6ba873..0e01b5af 100644
--- a/lib/cartodb/models/aggregation/aggregation-query.js
+++ b/lib/cartodb/models/aggregation/aggregation-query.js
@@ -119,83 +119,13 @@ const aggregateColumnDefs = ctx => {
 const aggregateDimensions = ctx => ctx.dimensions || {};
 
 const timeDimensionParameters = definition => {
-    let group_by_count = definition.step || 1;
-    let group_by_units;
-    let group_by_cycle;
-    switch (definition.group_by) {
-        case 'second':
-        case 'minute':
-        case 'hour':
-        case 'day':
-        case 'week':
-        case 'month':
-        case 'quarter':
-        case 'year':
-        case 'century':
-        case 'millenium':
-            group_by_units = definition.group_by;
-            break;
-        case 'semester':
-            group_by_units = 'month';
-            group_by_count *= 6;
-            break;
-        case 'trimester':
-            group_by_units = 'month';
-            group_by_count *= 4;
-            break;
-        case 'minuteOfHour':
-            group_by_units = 'minute';
-            group_by_cycle = 'hour';
-            break;
-        case 'hourOfDay':
-            group_by_units = 'hour';
-            group_by_cycle = 'day';
-            break;
-        case 'dayOfWeek':
-            group_by_units = 'day';
-            group_by_cycle = 'week';
-            break;
-        case 'dayOfMonth':
-            group_by_units = 'day';
-            group_by_cycle = 'month';
-            break;
-        case 'dayOfYear':
-            group_by_units = 'day';
-            group_by_cycle = 'year';
-            break;
-        case 'weekOfYear':
-            group_by_units = 'week';
-            group_by_cycle = 'year';
-            break;
-        case 'monthOfYear':
-            group_by_units = 'month';
-            group_by_cycle = 'year';
-            break;
-        case 'quarterOfYear':
-            group_by_units = 'quarter';
-            group_by_cycle = 'year';
-            break;
-        case 'trimesterOfYear':
-            group_by_units = 'month';
-            group_by_count *= 4;
-            group_by_cycle = 'year';
-            break;
-        case 'semesterOfYear':
-            group_by_units = 'month';
-            group_by_count *= 6;
-            group_by_cycle = 'year';
-            break;
-        default:
-            throw new Error(`Invalid time grouping ${definition.group_by}`);
-    }
     // definition.column should correspond to a wrapped date column
     return {
         time: `to_timestamp("${definition.column}")`,
         timezone: definition.timezone || 'utc',
-        group_by_units,
-        group_by_count,
-        group_by_cycle,
-        epoch: definition.starting
+        group_by: definition.group_by,
+        group_by_count: definition.group_by_count || 1,
+        starting: definition.starting
     };
 };
 
diff --git a/lib/cartodb/models/aggregation/time-dimension.js b/lib/cartodb/models/aggregation/time-dimension.js
index 8d39b671..63073c2f 100644
--- a/lib/cartodb/models/aggregation/time-dimension.js
+++ b/lib/cartodb/models/aggregation/time-dimension.js
@@ -1,24 +1,3 @@
-const MONTH_SECONDS = 365.2425 / 12 * 24 * 3600; // PG intervals use 30 * 24 * 3600
-const YEAR_SECONDS  = 12 * MONTH_SECONDS;
-
-// time unit durations
-const usecs = {
-    second: 1,
-    minute: 60,
-    hour: 3600,
-    day: 24 * 3600,
-    week: 7 * 24 * 3600,
-    month: MONTH_SECONDS,
-    year:  YEAR_SECONDS,
-
-    quarter: 3 * MONTH_SECONDS,
-    semester: 6 * MONTH_SECONDS,
-    trimester: 4 * MONTH_SECONDS,
-    decade: 12 * YEAR_SECONDS,
-    century: 100 * YEAR_SECONDS,
-    millennium: 1000 * YEAR_SECONDS
-};
-
 const YEARSPAN = "(date_part('year', $t)-date_part('year', $epoch))";
 // Note that SECONDSPAN is not a UTC epoch, but an epoch in the specified TZ,
 // so we can use it to compute any multiple of seconds with it  without using date_part or date_trunc
@@ -125,14 +104,14 @@ function serialNormalize(u, m) {
 }
 
 function cyclicNormalize(u, m, c) {
-    if (u === 'month' && m === 3) {
-        u = 'quarter';
+    if (u === 'monthOfYear' && m === 3) {
+        u = 'quarterOfYear';
         m = 1;
-    } else if (u === 'month' && m === 6) {
-        u = 'semester';
+    } else if (u === 'monthOfYear' && m === 6) {
+        u = 'semesterOfYear';
         m = 1;
-    } else if (u === 'month' && m === 4) {
-        u = 'trimester';
+    } else if (u === 'monthOfYear' && m === 4) {
+        u = 'trimesterOfYear';
         m = 1;
     }
     if (m !== 1) {
@@ -185,96 +164,89 @@ function epochExpression(epoch) {
     return `TIMESTAMP '${epoch}'`;
  }
 
-function cyclicSqlExpr(t, tz, u, c, m = 1) {
+function cyclicSqlExpr(t, tz, u, m = 1) {
     [u, m, c] = cyclicNormalize(u, m, c);
-    const comb = `${u}/${c}`;
     const column = timeExpression(t, tz);
 
     if (m === 1) {
-        switch (comb) {
-            case 'day/week':
+        switch (u) {
+            case 'dayOfWeek':
                 // 1 = monday; 7 = sunday;
                 return `date_part('isodow', ${column})`;
 
-            case 'day/month':
+            case 'dayOfMonth':
                 // result: 1-31
                 return `date_part('day', ${column})`;
 
-            case 'day/year':
+            case 'dayOfYear':
                 // result: 1-366
                 return `date_part('doy', ${column})`;
 
-            case 'hour/day':
+            case 'hourOfDay':
                 // result: 0-23
                 return `date_part('hour', ${column})`;
 
-            case 'month/year':
+            case 'monthOfYear':
                 // result 1-12
                 return `date_part('month', ${column})`;
 
-            case 'quarter/year':
+            case 'quarterOfYear':
                 // result 1-4
                 return `date_part('quarter', ${column})`;
 
-            case 'semester/year':
+            case 'semesterOfYear':
                 // result 1-2
                 return `FLOOR((date_part('month', ${column})-1)/6.0) + 1`;
 
-            case 'trimester/year':
+            case 'trimesterOfYear':
                 // result 1-3
                 return `FLOOR((date_part('month', ${column})-1)/4.0) + 1`;
 
-            case 'week/year':
+            case 'weekOfYear':
                 // result 1-53
                 return `date_part('week', ${column})`;
 
-            case 'minute/hour':
+            case 'minuteOfHour':
                 // result 0-59
                 return `date_part('minute', ${column})`;
         }
     }
-    // TODO: remove generic expression, reaching here should error
-    return genericCyclicSqlExpr(t, u, c, 0, m);
-}
-
-function genericCyclicSqlExpr(t, tz, u, c, c_offset = 0, m = 1) {
-    const usec = usecs[u];
-    const csec = usecs[c];
-    const column = timeExpression(t, tz);
-    return `((FLOOR(date_part('epoch', ${column})/(${usec*m}))*(${usec*m})+${c_offset}) % ${csec})/${usec*m}`;
+    throw new Error(`Invalid cyclic time grouping ${u}`)
 }
 
 function validateParameters(_params) {
     return true;
 }
 
+function isCyclic(groupBy) {
+    return groupBy.match(/.+By.+/);
+}
+
 function classificationSql(params) {
     validateParameters(params);
-    if (params.group_by_cycle) {
+    if (isCyclic(params.group_by)) {
         // TODO: validate group_by_count === 1, No epoch
         return cyclicSqlExpr(
             params.time,
-            params.timezone || 'utc',
-            params.group_by_units,
-            params.group_by_cycle,
-            0,
-            params.group_by_count || 1
+            params.timezone,
+            params.group_by,
+            params.group_by_count
         );
     } else if (params.format === 'iso') {
         // TODO: validate group_by_count === 1, No epoch
         return isoSqlExpr(
             params.time,
-            params.timezone || 'utc',
-            params.group_by_units,
-            params.group_by_count || 1
+            params.timezone,
+            params.group_by,
+            params.group_by_count
         );
     } else {
         return serialSqlExpr(
             params.time,
-            params.timezone || 'utc',
-            params.group_by_units,
-            params.group_by_count || 1,
-            params.epoch
+            params.timezone,
+            params.group_by,
+            params.group_by_count,
+            params.starting
         );
 
     }

From aff55351adb397e30396f80f4b2898b369cc0925 Mon Sep 17 00:00:00 2001
From: Javier Goizueta <jgoizueta@gmail.com>
Date: Wed, 3 Oct 2018 19:07:47 +0200
Subject: [PATCH 06/22] Unify parameter names

---
 .../models/aggregation/aggregation-query.js   |   6 +-
 .../models/aggregation/time-dimension.js      | 114 +++++++++---------
 2 files changed, 60 insertions(+), 60 deletions(-)

diff --git a/lib/cartodb/models/aggregation/aggregation-query.js b/lib/cartodb/models/aggregation/aggregation-query.js
index 0e01b5af..8933c876 100644
--- a/lib/cartodb/models/aggregation/aggregation-query.js
+++ b/lib/cartodb/models/aggregation/aggregation-query.js
@@ -122,9 +122,9 @@ const timeDimensionParameters = definition => {
     // definition.column should correspond to a wrapped date column
     return {
         time: `to_timestamp("${definition.column}")`,
-        timezone: definition.timezone || 'utc',
-        group_by: definition.group_by,
-        group_by_count: definition.group_by_count || 1,
+        timeZone: definition.timezone || 'utc',
+        groupBy: definition.group_by,
+        groupByCount: definition.group_by_count || 1,
         starting: definition.starting
     };
 };
diff --git a/lib/cartodb/models/aggregation/time-dimension.js b/lib/cartodb/models/aggregation/time-dimension.js
index 63073c2f..e70f9132 100644
--- a/lib/cartodb/models/aggregation/time-dimension.js
+++ b/lib/cartodb/models/aggregation/time-dimension.js
@@ -39,18 +39,18 @@ const serialParts = {
     }
 };
 
-function serialSqlExpr(t, tz, u, m = 1, starting = undefined) {
-    [u, m] = serialNormalize(u, m);
-    let { sql, zeroBased } = serialParts[u];
-    const column = timeExpression(t, tz);
+function serialSqlExpr(time, timeZone, groupBy, count = 1, starting = undefined) {
+    [groupBy, count] = serialNormalize(groupBy, count);
+    let { sql, zeroBased } = serialParts[groupBy];
+    const column = timeExpression(time, timeZone);
     const epoch  = epochExpression(starting);
     const serial = sql.replace(/\$t/g, column).replace(/\$epoch/g, epoch);
     let expr = serial;
-    if (m !== 1) {
+    if (count !== 1) {
         if (zeroBased) {
-            expr = `FLOOR((${expr})/(${m}::double precision))::int`;
+            expr = `FLOOR((${expr})/(${count}::double precision))::int`;
         } else {
-            expr = `CEIL((${expr})/(${m}::double precision))::int`;
+            expr = `CEIL((${expr})/(${count}::double precision))::int`;
         }
     } else {
         expr = `(${expr})::int`;
@@ -74,50 +74,50 @@ const isoParts = {
     millennium: `to_char(date_part('millenium', $t), '"M"999')`
 };
 
-function isoSqlExpr(t, tz, u, m = 1) {
-    const column = timeExpression(t, tz);
-    if (m > 1) {
+function isoSqlExpr(time, timeZone, groupBy, count = 1) {
+    const column = timeExpression(time, timeZone);
+    if (count > 1) {
         // TODO: it would be sensible to return the ISO of the firt unit in the period
         throw new Error('Multiple time units not supported for ISO format');
     }
-    return isoParts[u].replace(/\$t/g, column);
+    return isoParts[groupBy].replace(/\$t/g, column);
 }
 
-function serialNormalize(u, m) {
-    if (u === 'semester') {
-        u = 'month';
-        m *= 6;
-    } else if (u === 'trimester') {
-        u = 'month';
-        m *= 4;
-    } else if (u === 'decade') {
-        u = 'year';
-        m *= 10;
-    } else if (u === 'century') {
-        u = 'year';
-        m *= 100;
-    } else if (u === 'millenium') {
-        u = 'year';
-        m *= 1000;
+function serialNormalize(groupBy, count) {
+    if (groupBy === 'semester') {
+        groupBy = 'month';
+        count *= 6;
+    } else if (groupBy === 'trimester') {
+        groupBy = 'month';
+        count *= 4;
+    } else if (groupBy === 'decade') {
+        groupBy = 'year';
+        count *= 10;
+    } else if (groupBy === 'century') {
+        groupBy = 'year';
+        count *= 100;
+    } else if (groupBy === 'millenium') {
+        groupBy = 'year';
+        count *= 1000;
     }
-    return [u, m];
+    return [groupBy, count];
 }
 
-function cyclicNormalize(u, m, c) {
-    if (u === 'monthOfYear' && m === 3) {
-        u = 'quarterOfYear';
-        m = 1;
-    } else if (u === 'monthOfYear' && m === 6) {
-        u = 'semesterOfYear';
-        m = 1;
-    } else if (u === 'monthOfYear' && m === 4) {
-        u = 'trimesterOfYear';
-        m = 1;
+function cyclicNormalize(groupBy, count) {
+    if (groupBy === 'monthOfYear' && count === 3) {
+        groupBy = 'quarterOfYear';
+        count = 1;
+    } else if (groupBy === 'monthOfYear' && count === 6) {
+        groupBy = 'semesterOfYear';
+        count = 1;
+    } else if (groupBy === 'monthOfYear' && count === 4) {
+        groupBy = 'trimesterOfYear';
+        count = 1;
     }
-    if (m !== 1) {
-        throw new Error(`invalid multiplicity ${m} for cyclic ${u}`);
+    if (count !== 1) {
+        throw new Error(`invalid multiplicity ${count} for cyclic ${groupBy}`);
     }
-    return [u, m, c];
+    return [groupBy, count];
 }
 
 // timezones can be defined either by an numeric offset in seconds or by
@@ -159,17 +159,17 @@ function epochExpression(epoch) {
     const day = match[3] || '01';
     const hour = match[4] || '00';
     const minute = match[5] || '00';
-    const second = match[6] || '00';
+    const second = match[6]t || '00';
     epoch = `${year}-${month}-${day}T${hour}:${minute}:${second}`;
     return `TIMESTAMP '${epoch}'`;
  }
 
-function cyclicSqlExpr(t, tz, u, m = 1) {
-    [u, m, c] = cyclicNormalize(u, m, c);
-    const column = timeExpression(t, tz);
+function cyclicSqlExpr(time, timeZone, groupBy, count = 1) {
+    [groupBy, count] = cyclicNormalize(groupBy, count);
+    const column = timeExpression(time, timeZone);
 
-    if (m === 1) {
-        switch (u) {
+    if (count === 1) {
+        switch (groupBy) {
             case 'dayOfWeek':
                 // 1 = monday; 7 = sunday;
                 return `date_part('isodow', ${column})`;
@@ -211,7 +211,7 @@ function cyclicSqlExpr(t, tz, u, m = 1) {
                 return `date_part('minute', ${column})`;
         }
     }
-    throw new Error(`Invalid cyclic time grouping ${u}`)
+    throw new Error(`Invalid cyclic time grouping ${groupBy} with count ${count}`)
 }
 
 function validateParameters(_params) {
@@ -228,24 +228,24 @@ function classificationSql(params) {
         // TODO: validate group_by_count === 1, No epoch
         return cyclicSqlExpr(
             params.time,
-            params.timezone,
-            params.group_by,
-            params.group_by_count
+            params.timeZone,
+            params.groupBy,
+            params.groupByCount
         );
     } else if (params.format === 'iso') {
         // TODO: validate group_by_count === 1, No epoch
         return isoSqlExpr(
             params.time,
-            params.timezone,
-            params.group_by,
-            params.group_by_count
+            params.timeZone,
+            params.groupBy,
+            params.groupByCount
         );
     } else {
         return serialSqlExpr(
             params.time,
-            params.timezone,
-            params.group_by,
-            params.group_by_count,
+            params.timeZone,
+            params.groupBy,
+            params.groupByCount,
             params.starting
         );
 

From c588d4139ea0efe350a2192c465f5ba7b7a3d7e3 Mon Sep 17 00:00:00 2001
From: Javier Goizueta <jgoizueta@gmail.com>
Date: Wed, 3 Oct 2018 21:02:22 +0200
Subject: [PATCH 07/22] Refactor time dimensions

---
 .../models/aggregation/aggregation-query.js   |   9 +-
 .../models/aggregation/time-dimension.js      | 352 ++++++++++--------
 2 files changed, 193 insertions(+), 168 deletions(-)

diff --git a/lib/cartodb/models/aggregation/aggregation-query.js b/lib/cartodb/models/aggregation/aggregation-query.js
index 8933c876..792a1bd7 100644
--- a/lib/cartodb/models/aggregation/aggregation-query.js
+++ b/lib/cartodb/models/aggregation/aggregation-query.js
@@ -122,10 +122,11 @@ const timeDimensionParameters = definition => {
     // definition.column should correspond to a wrapped date column
     return {
         time: `to_timestamp("${definition.column}")`,
-        timeZone: definition.timezone || 'utc',
-        groupBy: definition.group_by,
-        groupByCount: definition.group_by_count || 1,
-        starting: definition.starting
+        timezone: definition.timezone || 'utc',
+        grouping: definition.group_by,
+        count: definition.group_by_count || 1,
+        starting: definition.starting,
+        format: definition.format
     };
 };
 
diff --git a/lib/cartodb/models/aggregation/time-dimension.js b/lib/cartodb/models/aggregation/time-dimension.js
index e70f9132..6fb09771 100644
--- a/lib/cartodb/models/aggregation/time-dimension.js
+++ b/lib/cartodb/models/aggregation/time-dimension.js
@@ -1,3 +1,48 @@
+// timezones can be defined either by an numeric offset in seconds or by
+// a valid (case-insensitive) tz/PG name;
+// they include abbreviations defined by PG (which have precedence and
+// are fixed offsets, not handling DST) or general names that can handle DST.
+function timezone(tz) {
+    if (isFinite(tz)) {
+        return `INTERVAL '${tz} seconds'`;
+    }
+    return `'${tz}'`;
+}
+
+// We assume t is a TIMESTAMP WITH TIME ZONE.
+// If this was to be used with a t which is a TIMESTAMP or TIME (no time zone)
+// it should be converted with `timezone('utc',t)` to a type with time zone.
+// Note that by default CARTO uses timestamp with time zone columns for dates
+// and VectorMapConfigAdapter converts them to epoch numbers.
+// So, for using this with aggregations, relying on dates & times
+// converted to UTC UNIX epoch numbers, apply `to_timestamp` to the
+// (converted) column.
+function timeExpression(t, tz) {
+   if (tz !== undefined) {
+        return `timezone(${timezone(tz)}, ${t})`;
+   }
+   return t;
+}
+
+// Epoch should be an ISO timestamp literal without time zone
+// (it is interpreted as in the defined timzezone for the input time)
+// It can be partial, e.g. 'YYYY', 'YYYY-MM', 'YYYY-MM-DDTHH', etc.
+// Defaults are applied: YYYY=0001, MM=01, DD=01, HH=00, MM=00, S=00
+// It returns a timestamp without time zone
+function epochExpression(epoch) {
+    /* jshint maxcomplexity:8 */ // goddammit linter, I like this as is!!
+    const format = /^(\d\d\d\d)(?:\-?(\d\d)(?:\-?(\d\d)(?:[T\s]?(\d\d)(?:(\d\d)(?:\:(\d\d))?)?)?)?)?$/;
+    const match  = epoch.match(format) || [];
+    const year   = match[1] || '0001';
+    const month  = match[2] || '01';
+    const day    = match[3] || '01';
+    const hour   = match[4] || '00';
+    const minute = match[5] || '00';
+    const second = match[6] || '00';
+    epoch = `${year}-${month}-${day}T${hour}:${minute}:${second}`;
+    return `TIMESTAMP '${epoch}'`;
+ }
+
 const YEARSPAN = "(date_part('year', $t)-date_part('year', $epoch))";
 // Note that SECONDSPAN is not a UTC epoch, but an epoch in the specified TZ,
 // so we can use it to compute any multiple of seconds with it  without using date_part or date_trunc
@@ -32,25 +77,47 @@ const serialParts = {
         sql: `1 + date_part('quarter', $t) - date_part('quarter', $epoch) + 4*${YEARSPAN}`,
         zeroBased: false
     },
+    semester: {
+        sql: `1 + FLOOR((date_part('month', $t) - date_part('month', $epoch))/6) + 2*${YEARSPAN}`,
+        zeroBased: false
+    },
+    trimester: {
+        sql: `1 + FLOOR((date_part('month', $t) - date_part('month', $epoch))/4) + 3*${YEARSPAN}`,
+        zeroBased: false
+    },
     year: {
-        // TODO: isn't more meaningful to ignore the epoch here and return date_part('year', $t)
+        // for the default epoch this coincides with date_part('year', $t)
         sql: `1 + ${YEARSPAN}`,
         zeroBased: false
+    },
+    decade: {
+        // for the default epoch this coincides with date_part('decade', $t)
+        sql: `FLOOR((${YEARSPAN} + 1)/10)`,
+        zeroBased: true
+    },
+    century: {
+        // for the default epoch this coincides with date_part('century', $t)
+        sql: `1 + FLOOR(${YEARSPAN}/100)`,
+        zeroBased: false
+    },
+    millennium: {
+        // for the default epoch this coincides with date_part('millennium', $t)
+        sql: `1 + FLOOR(${YEARSPAN}/1000)`,
+        zeroBased: false
     }
 };
 
-function serialSqlExpr(time, timeZone, groupBy, count = 1, starting = undefined) {
-    [groupBy, count] = serialNormalize(groupBy, count);
-    let { sql, zeroBased } = serialParts[groupBy];
-    const column = timeExpression(time, timeZone);
-    const epoch  = epochExpression(starting);
+function serialSqlExpr(params) {
+    const { sql, zeroBased } = serialParts[params.grouping];
+    const column = timeExpression(params.time, params.timezone);
+    const epoch  = epochExpression(params.starting);
     const serial = sql.replace(/\$t/g, column).replace(/\$epoch/g, epoch);
     let expr = serial;
-    if (count !== 1) {
+    if (params.count !== 1) {
         if (zeroBased) {
-            expr = `FLOOR((${expr})/(${count}::double precision))::int`;
+            expr = `FLOOR((${expr})/(${params.count}::double precision))::int`;
         } else {
-            expr = `CEIL((${expr})/(${count}::double precision))::int`;
+            expr = `CEIL((${expr})/(${params.count}::double precision))::int`;
         }
     } else {
         expr = `(${expr})::int`;
@@ -71,184 +138,141 @@ const isoParts = {
     trimester: `to_char($t, 'YYYY"t"') || to_char(CEIL(date_part('month', $t)/4), '9')`,
     decade: `to_char(date_part('decade', $t), '"D"999')`,
     century: `to_char($t, '"C"CC')`,
-    millennium: `to_char(date_part('millenium', $t), '"M"999')`
+    millennium: `to_char(date_part('millennium', $t), '"M"999')`
 };
 
-function isoSqlExpr(time, timeZone, groupBy, count = 1) {
-    const column = timeExpression(time, timeZone);
-    if (count > 1) {
-        // TODO: it would be sensible to return the ISO of the firt unit in the period
+function isoSqlExpr(params) {
+    const column = timeExpression(params.time, params.timezone);
+    if (params.count > 1) {
+        // TODO: it would be sensible to return the ISO of the first unit in the period
         throw new Error('Multiple time units not supported for ISO format');
     }
-    return isoParts[groupBy].replace(/\$t/g, column);
+    return isoParts[params.grouping].replace(/\$t/g, column);
 }
 
-function serialNormalize(groupBy, count) {
-    if (groupBy === 'semester') {
-        groupBy = 'month';
-        count *= 6;
-    } else if (groupBy === 'trimester') {
-        groupBy = 'month';
-        count *= 4;
-    } else if (groupBy === 'decade') {
-        groupBy = 'year';
-        count *= 10;
-    } else if (groupBy === 'century') {
-        groupBy = 'year';
-        count *= 100;
-    } else if (groupBy === 'millenium') {
-        groupBy = 'year';
-        count *= 1000;
+const cyclicParts = {
+    dayOfWeek: `date_part('isodow', $t)`,                         // 1 = monday to 7 = sunday;
+    dayOfMonth: `date_part('day', $t)`,                           // 1 to 31
+    dayOfYear: `date_part('doy', $t)`,                            // 1 to 366
+    hourOfDay: `date_part('hour', $t)`,                           // 0 to 23
+    monthOfYear: `date_part('month', $t)`,                        // 1 to 12
+    quarterOfYear: `date_part('quarter', $t)`,                    // 1 to 4
+    semesterOfYear: `FLOOR((date_part('month', $t)-1)/6.0) + 1`,  // 1 to 2
+    trimesterOfYear: `FLOOR((date_part('month', $t)-1)/4.0) + 1`, // 1 to 3
+    weekOfYear: `date_part('week', $t)`,                          // 1 to 53
+    minuteOfHour: `date_part('minute', $t)`                       // 0 to 59
+};
+
+function cyclicSqlExpr(params) {
+    const column = timeExpression(params.time, params.timezone);
+    return isoParts[params.grouping].replace(/\$t/g, column);
+}
+
+const ACCEPTED_PARAMETERS = ['time', 'grouping', 'timezone', 'count', 'starting', 'format'];
+const REQUIRED_PARAMETERS = ['time', 'grouping'];
+
+function validateParameters(params, checker) {
+    const errors = [];
+    const presentParams = Object.keys(params);
+    const invalidParams = presentParams.filter(param => !ACCEPTED_PARAMETERS.includes(param));
+    if (invalidParams.length) {
+        errors.push(`Invalid parameters: ${invalidParams.join(', ')}`);
     }
-    return [groupBy, count];
-}
-
-function cyclicNormalize(groupBy, count) {
-    if (groupBy === 'monthOfYear' && count === 3) {
-        groupBy = 'quarterOfYear';
-        count = 1;
-    } else if (groupBy === 'monthOfYear' && count === 6) {
-        groupBy = 'semesterOfYear';
-        count = 1;
-    } else if (groupBy === 'monthOfYear' && count === 4) {
-        groupBy = 'trimesterOfYear';
-        count = 1;
+    const missingParams = REQUIRED_PARAMETERS.filter(param => !presentParams.includes(param));
+    if (missingParams.length) {
+        errors.push(`Missing parameters: ${missingParams.join(', ')}`);
     }
-    if (count !== 1) {
-        throw new Error(`invalid multiplicity ${count} for cyclic ${groupBy}`);
+    errors.push(...checker(params));
+    if (errors.length) {
+        throw new Error(`Invalid time dimension:\n${errors.join("\n")}`);
     }
-    return [groupBy, count];
 }
 
-// timezones can be defined either by an numeric offset in seconds or by
-// a valid (case-insensitive) tz/PG name;
-// they include abbreviations defined by PG (which have precedence and
-// are fixed offsets, not handling DST) or general names that can handle DST.
-function timezone(tz) {
-    if (isFinite(tz)) {
-        return `INTERVAL '${tz} seconds'`;
-    }
-    return `'${tz}'`;
-}
+const VALID_CYCLIC_GROUPINGS = Object.keys(cyclicParts);
+const VALID_SERIAL_GROUPINGS = Object.keys(serialParts);
+const VALID_ISO_GROUPINGS = Object.keys(isoParts);
 
-// We assume t is a TIMESTAMP WITH TIME ZONE.
-// If this was to be used with a t which is a TIMESTAMP or TIME (no time zone)
-// it should be converted with `timezone('utc',t)` to a type with time zone.
-// Note that by default CARTO uses timestamp with time zone columns for dates
-// and VectorMapConfigAdapter converts them to epoch numbers.
-// So, for using this with aggregations, relying on dates & times
-// converted to UTC UNIX epoch numbers, apply `to_timestamp` to the
-// (converted) column.
-function timeExpression(t, tz) {
-   if (tz !== undefined) {
-        return `timezone(${timezone(tz)}, ${t})`;
-   }
-   return t;
-}
+const MONTH_GROUPING = {
+  3: 'quarterOfYear',
+  6: 'semesterOfYear',
+  4: 'trimesterOfYear'
+};
 
-// Epoch should be an ISO timestamp literal without time zone
-// (it is interpreted as in the defined timzezone for the input time)
-// It can be partial, e.g. 'YYYY', 'YYYY-MM', 'YYYY-MM-DDTHH', etc.
-// Defaults are applied: YYYY=0001, MM=01, DD=01, HH=00, MM=00, S=00
-// It returns a timestamp without time zone
-function epochExpression(epoch) {
-    const format = /^(\d\d\d\d)(?:\-?(\d\d)(?:\-?(\d\d)(?:[T\s]?(\d\d)(?:(\d\d)(?:\:(\d\d))?)?)?)?)?$/;
-    const match = epoch.match(format) || [];
-    const year = match[1] || '0001';
-    const month = match[2] || '01';
-    const day = match[3] || '01';
-    const hour = match[4] || '00';
-    const minute = match[5] || '00';
-    const second = match[6]t || '00';
-    epoch = `${year}-${month}-${day}T${hour}:${minute}:${second}`;
-    return `TIMESTAMP '${epoch}'`;
- }
-
-function cyclicSqlExpr(time, timeZone, groupBy, count = 1) {
-    [groupBy, count] = cyclicNormalize(groupBy, count);
-    const column = timeExpression(time, timeZone);
-
-    if (count === 1) {
-        switch (groupBy) {
-            case 'dayOfWeek':
-                // 1 = monday; 7 = sunday;
-                return `date_part('isodow', ${column})`;
-
-            case 'dayOfMonth':
-                // result: 1-31
-                return `date_part('day', ${column})`;
-
-            case 'dayOfYear':
-                // result: 1-366
-                return `date_part('doy', ${column})`;
-
-            case 'hourOfDay':
-                // result: 0-23
-                return `date_part('hour', ${column})`;
-
-            case 'monthOfYear':
-                // result 1-12
-                return `date_part('month', ${column})`;
-
-            case 'quarterOfYear':
-                // result 1-4
-                return `date_part('quarter', ${column})`;
-
-            case 'semesterOfYear':
-                // result 1-2
-                return `FLOOR((date_part('month', ${column})-1)/6.0) + 1`;
-
-            case 'trimesterOfYear':
-                // result 1-3
-                return `FLOOR((date_part('month', ${column})-1)/4.0) + 1`;
-
-            case 'weekOfYear':
-                // result 1-53
-                return `date_part('week', ${column})`;
-
-            case 'minuteOfHour':
-                // result 0-59
-                return `date_part('minute', ${column})`;
+function cyclicCheckParams(params) {
+    const errors = [];
+    if (!VALID_CYCLIC_GROUPINGS.includes(params.grouping)) {
+        errors.push(`Invalid grouping "${params.grouping}"`);
+    } else {
+        if (params.count && params.count > 1) {
+            let fixed = false;
+            if (params.grouping === 'monthOfYear') {
+                 const grouping = MONTH_GROUPING[params.count];
+                 if (grouping) {
+                     params.grouping = grouping;
+                     params.count = 1;
+                     fixed = true;
+                 }
+            }
+            if (!fixed) {
+                errors.push(`Invalid count ${params.count} for cyclic ${params.grouping}`);
+            }
         }
     }
-    throw new Error(`Invalid cyclic time grouping ${groupBy} with count ${count}`)
+    return errors;
 }
 
-function validateParameters(_params) {
-    return true;
+function serialCheckParams(params) {
+    const errors = [];
+    if (!VALID_SERIAL_GROUPINGS.includes(params.grouping)) {
+        errors.push(`Invalid grouping "${params.grouping}"`);
+    }
+    return errors;
 }
 
+function isoCheckParams(params) {
+    const errors = [];
+    if (!VALID_ISO_GROUPINGS.includes(params.grouping)) {
+        errors.push(`Invalid grouping "${params.grouping}"`);
+    }
+    if (params.starting) {
+        errors.push("Parameter 'starting' not supported for ISO format");
+    }
+    return errors;
+}
+
+const CLASSIFIERS = {
+    cyclic: {
+        sqlExpr: cyclicSqlExpr,
+        checkParams: cyclicCheckParams
+    },
+    iso: {
+        sqlExpr: isoSqlExpr,
+        checkParams: isoCheckParams
+    },
+    serial: {
+        sqlExpr: serialSqlExpr,
+        checkParams: serialCheckParams
+    }
+};
+
 function isCyclic(groupBy) {
     return groupBy.match(/.+By.+/);
 }
 
-function classificationSql(params) {
-    validateParameters(params);
-    if (isCyclic(params.group_by)) {
-        // TODO: validate group_by_count === 1, No epoch
-        return cyclicSqlExpr(
-            params.time,
-            params.timeZone,
-            params.groupBy,
-            params.groupByCount
-        );
+function classifierFor(params) {
+    let classifier = 'serial';
+    if (params.grouping && isCyclic(params.grouping)) {
+        classifier = 'cyclic';
     } else if (params.format === 'iso') {
-        // TODO: validate group_by_count === 1, No epoch
-        return isoSqlExpr(
-            params.time,
-            params.timeZone,
-            params.groupBy,
-            params.groupByCount
-        );
-    } else {
-        return serialSqlExpr(
-            params.time,
-            params.timeZone,
-            params.groupBy,
-            params.groupByCount,
-            params.starting
-        );
-
+        classifier = 'iso';
     }
+    return CLASSIFIERS[classifier];
 }
-module.exports = classificationSql;
+
+function classificationSql(params) {
+    const classifier = classifierFor(params);
+    validateParameters(params, classifier.checkParams);
+    return classifier.sqlExpr(params);
+}
+
+module.exports = classificationSql;
\ No newline at end of file

From 99b62edcbdbe2c30c74a426d4b77932c67f43dce Mon Sep 17 00:00:00 2001
From: Javier Goizueta <jgoizueta@gmail.com>
Date: Wed, 3 Oct 2018 23:12:58 +0200
Subject: [PATCH 08/22] Bug fixes

---
 lib/cartodb/models/aggregation/time-dimension.js | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/cartodb/models/aggregation/time-dimension.js b/lib/cartodb/models/aggregation/time-dimension.js
index 6fb09771..1a1909ed 100644
--- a/lib/cartodb/models/aggregation/time-dimension.js
+++ b/lib/cartodb/models/aggregation/time-dimension.js
@@ -32,7 +32,7 @@ function timeExpression(t, tz) {
 function epochExpression(epoch) {
     /* jshint maxcomplexity:8 */ // goddammit linter, I like this as is!!
     const format = /^(\d\d\d\d)(?:\-?(\d\d)(?:\-?(\d\d)(?:[T\s]?(\d\d)(?:(\d\d)(?:\:(\d\d))?)?)?)?)?$/;
-    const match  = epoch.match(format) || [];
+    const match  = (epoch || '').match(format) || [];
     const year   = match[1] || '0001';
     const month  = match[2] || '01';
     const day    = match[3] || '01';
@@ -165,7 +165,7 @@ const cyclicParts = {
 
 function cyclicSqlExpr(params) {
     const column = timeExpression(params.time, params.timezone);
-    return isoParts[params.grouping].replace(/\$t/g, column);
+    return cyclicParts[params.grouping].replace(/\$t/g, column);
 }
 
 const ACCEPTED_PARAMETERS = ['time', 'grouping', 'timezone', 'count', 'starting', 'format'];
@@ -256,7 +256,7 @@ const CLASSIFIERS = {
 };
 
 function isCyclic(groupBy) {
-    return groupBy.match(/.+By.+/);
+    return VALID_CYCLIC_GROUPINGS.includes(groupBy);
 }
 
 function classifierFor(params) {

From c9786ee3f63826e4899bda453a3f06cc8068f29c Mon Sep 17 00:00:00 2001
From: Javier Goizueta <jgoizueta@gmail.com>
Date: Wed, 3 Oct 2018 23:13:22 +0200
Subject: [PATCH 09/22] Catch aggregation query errors

---
 .../mapconfig/adapter/aggregation-mapconfig-adapter.js   | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js b/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js
index 9752ab57..82de77d1 100644
--- a/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js
+++ b/lib/cartodb/models/mapconfig/adapter/aggregation-mapconfig-adapter.js
@@ -95,7 +95,14 @@ module.exports = class AggregationMapConfigAdapter {
 
                 const sqlQueryWrap = layer.options.sql_wrap;
 
-                let aggregationSql = mapConfig.getAggregatedQuery(index);
+                let aggregationSql;
+
+                try {
+                    aggregationSql = mapConfig.getAggregatedQuery(index);
+                }
+                catch (error) {
+                    return reject(error);
+                }
 
                 if (sqlQueryWrap) {
                     aggregationSql = sqlQueryWrap.replace(/<%=\s*sql\s*%>/g, aggregationSql);

From f841f65a1e78b8e64593f8bf1fc95eb786fa1975 Mon Sep 17 00:00:00 2001
From: Javier Goizueta <jgoizueta@gmail.com>
Date: Thu, 4 Oct 2018 19:50:14 +0200
Subject: [PATCH 10/22] Dimensions metadata

---
 .../layer-stats/mapnik-layer-stats.js         | 108 ++++++++++++------
 .../models/aggregation/aggregation-query.js   |  62 ++++++----
 .../models/aggregation/time-dimension.js      |  57 ++++-----
 3 files changed, 136 insertions(+), 91 deletions(-)

diff --git a/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js b/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js
index 85eeecb2..fb62d532 100644
--- a/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js
+++ b/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js
@@ -1,5 +1,7 @@
 const queryUtils = require('../../utils/query-utils');
 const AggregationMapConfig = require('../../models/aggregation/aggregation-mapconfig');
+const aggregationQuery = require('../../models/aggregation/aggregation-query');
+
 
 function MapnikLayerStats () {
     this._types = {
@@ -19,6 +21,9 @@ function columnAggregations(field) {
     if (field.type === 'date') { // TODO other types too?
         return ['min', 'max'];
     }
+    if (field.type === 'timeDimension') {
+        return ['min', 'max'];
+    }
     return [];
 }
 
@@ -137,35 +142,53 @@ function _sample(ctx, numRows) {
     return Promise.resolve();
 }
 
-function _columnStats(ctx, columns) {
+function _columnStats(ctx, columns, dimensions) {
     if (!columns) {
         return Promise.resolve();
     }
-    if (ctx.metaOptions.columnStats) {
+    if (ctx.metaOptions.columnStats || ctx.metaOptions.dimensions) {
         let queries = [];
         let aggr = [];
-        queries.push(new Promise(resolve => resolve(columns))); // add columns as first result
-        Object.keys(columns).forEach(name => {
-            aggr = aggr.concat(
-                columnAggregations(columns[name])
-                .map(fn => `${fn}("${name}") AS "${name}_${fn}"`)
-            );
-            if (columns[name].type === 'string') {
-                const topN = ctx.metaOptions.columnStats.topCategories || 1024;
-                const includeNulls = ctx.metaOptions.columnStats.hasOwnProperty('includeNulls') ?
-                    ctx.metaOptions.columnStats.includeNulls :
-                    true;
-
-                // TODO:  ctx.metaOptions.columnStats.maxCategories
-                //        => use PG stats to dismiss columns with more distinct values
-                queries.push(
-                    queryUtils.queryPromise(
-                        ctx.dbConnection,
-                        _getSQL(ctx, sql => queryUtils.getQueryTopCategories(sql, name, topN, includeNulls))
-                    ).then(res => ({ [name]: { categories: res.rows } }))
+        if (ctx.metaOptions.columnStats) {
+            queries.push(new Promise(resolve => resolve(columns))); // add columns as first result
+            Object.keys(columns).forEach(name => {
+                aggr = aggr.concat(
+                    columnAggregations(columns[name])
+                    .map(fn => `${fn}("${name}") AS "${name}_${fn}"`)
                 );
-            }
-        });
+                if (columns[name].type === 'string') {
+                    const topN = ctx.metaOptions.columnStats.topCategories || 1024;
+                    const includeNulls = ctx.metaOptions.columnStats.hasOwnProperty('includeNulls') ?
+                        ctx.metaOptions.columnStats.includeNulls :
+                        true;
+
+                    // TODO:  ctx.metaOptions.columnStats.maxCategories
+                    //        => use PG stats to dismiss columns with more distinct values
+                    queries.push(
+                        queryUtils.queryPromise(
+                            ctx.dbConnection,
+                            _getSQL(ctx, sql => queryUtils.getQueryTopCategories(sql, name, topN, includeNulls))
+                        ).then(res => ({ [name]: { categories: res.rows } }))
+                    );
+                }
+            });
+        }
+        const dimensionsStats = {};
+        let dimensionsInfo = {};
+        if (ctx.metaOptions.dimensions && dimensions) {
+            dimensionsInfo = aggregationQuery.infoForOptions({ dimensions });
+            Object.keys(dimensionsInfo).forEach(dimName => {
+                const info = dimensionsInfo[dimName];
+                if (info.type === 'timeDimension') {
+                    dimensionsStats[dimName] = {
+                        params: info.params
+                    };
+                    aggr = aggr.concat(
+                        columnAggregations(info).map(fn => `${fn}(${info.sql}) AS "${dimName}_${fn}"`)
+                    );
+                }
+            });
+        }
         queries.push(
             queryUtils.queryPromise(
                 ctx.dbConnection,
@@ -178,6 +201,17 @@ function _columnStats(ctx, columns) {
                         stats[name][fn] = res.rows[0][`${name}_${fn}`];
                     });
                 });
+                Object.keys(dimensionsInfo).forEach(name => {
+                    // Temporalily place dimensions info in stats.columns.__dimensions
+                    stats.__dimensions = stats.__dimensions || {};
+                    stats.__dimensions[name] = stats.__dimensions[name] || Object.assign({}, dimensionsStats[name]);
+                    let type = null;
+                    columnAggregations(dimensionsInfo[name]).forEach(fn => {
+                        type = type || fieldTypeSafe(ctx.dbConnection, res.fields.find(f => f.name === `${name}_${fn}`));
+                        stats.__dimensions[name][fn] = res.rows[0][`${name}_${fn}`];
+                    });
+                    stats.__dimensions[name].type = type;
+                });
                 return stats;
             })
         );
@@ -211,19 +245,17 @@ function fieldType(cname) {
     return tname;
 }
 
+function fieldTypeSafe(dbConnection, field) {
+    const cname = dbConnection.typeName(field.dataTypeID);
+    return cname ? fieldType(cname) : `unknown(${field.dataTypeID})`;
+}
+
 // columns are returned as an object { columnName1: { type1: ...}, ..}
 // for consistency with SQL API
 function formatResultFields(dbConnection, fields = []) {
     let nfields = {};
     for (let field of fields) {
-      const cname = dbConnection.typeName(field.dataTypeID);
-      let tname;
-      if ( ! cname ) {
-        tname = 'unknown(' + field.dataTypeID + ')';
-      } else {
-        tname = fieldType(cname);
-      }
-      nfields[field.name] = { type: tname };
+      nfields[field.name] = { type: fieldTypeSafe(dbConnection, field) };
     }
     return nfields;
 }
@@ -237,7 +269,7 @@ function (layer, dbConnection, callback) {
         dbConnection,
         preQuery,
         aggrQuery,
-        metaOptions: layer.options.metadata || {}
+        metaOptions: layer.options.metadata || {},
     };
 
     // TODO: could save some queries if queryUtils.getAggregationMetadata() has been used and kept somewhere
@@ -248,6 +280,8 @@ function (layer, dbConnection, callback) {
     // TODO: add support for sample.exclude option by, in that case, forcing the columns query and
     // passing the results to the sample query function.
 
+    const dimensions = (layer.options.aggregation || {}).dimensions;
+
     Promise.all([
         _estimatedFeatureCount(ctx).then(
             ({ estimatedFeatureCount }) => _sample(ctx, estimatedFeatureCount)
@@ -256,9 +290,15 @@ function (layer, dbConnection, callback) {
         _featureCount(ctx),
         _aggrFeatureCount(ctx),
         _geometryType(ctx),
-        _columns(ctx).then(columns => _columnStats(ctx, columns))
+        _columns(ctx).then(columns => _columnStats(ctx, columns, dimensions))
     ]).then(results => {
-        callback(null, mergeResults(results));
+        results = mergeResults(results);
+        const dimensions = results.columns && results.columns.__dimensions;
+        if (dimensions) {
+            delete results.columns.__dimensions;
+            results.dimensions = dimensions;
+        }
+        callback(null, results);
     }).catch(error => {
         callback(error);
     });
diff --git a/lib/cartodb/models/aggregation/aggregation-query.js b/lib/cartodb/models/aggregation/aggregation-query.js
index 792a1bd7..8f8d2de8 100644
--- a/lib/cartodb/models/aggregation/aggregation-query.js
+++ b/lib/cartodb/models/aggregation/aggregation-query.js
@@ -27,6 +27,16 @@ const templateForOptions = (options) => {
     return templateFn;
 };
 
+function optionsToParams (options) {
+    return {
+        sourceQuery: options.query,
+        res: 256/options.resolution,
+        columns: options.columns,
+        dimensions: options.dimensions,
+        filters: options.filters
+    };
+}
+
 /**
  * Generates an aggregation query given the aggregation options:
  * - query
@@ -41,18 +51,23 @@ const templateForOptions = (options) => {
  * When placement, columns or dimensions are specified, columns are aggregated as requested
  * (by default only _cdb_feature_count) and with the_geom_webmercator as defined by placement.
  */
-const queryForOptions = (options) => {
-    return templateForOptions(options)({
-    sourceQuery: options.query,
-    res: 256/options.resolution,
-    columns: options.columns,
-    dimensions: options.dimensions,
-    filters: options.filters
-    });
-};
+const queryForOptions = (options) => templateForOptions(options)(optionsToParams(options));
 
 module.exports = queryForOptions;
 
+module.exports.infoForOptions = (options) => {
+    const params = optionsToParams(options);
+    const dimensions = {};
+    dimensionNamesAndExpressions(params).forEach(([dimensionName, info]) => {
+        dimensions[dimensionName] = {
+            sql: info.sql,
+            params: info.effectiveParams,
+            type: info.type
+        };
+    });
+    return dimensions;
+};
+
 const SUPPORTED_AGGREGATE_FUNCTIONS = {
     'count': {
         sql: (column_name, params) => `count(${params.aggregated_column || '*'})`
@@ -123,8 +138,8 @@ const timeDimensionParameters = definition => {
     return {
         time: `to_timestamp("${definition.column}")`,
         timezone: definition.timezone || 'utc',
-        grouping: definition.group_by,
-        count: definition.group_by_count || 1,
+        grouping: definition.grouping,
+        count: definition.count || 1,
         starting: definition.starting,
         format: definition.format
     };
@@ -139,28 +154,31 @@ const adaptDimensionDefinition = definition => {
 };
 
 const dimensionExpression = definition => {
-    if (definition.group_by) {
+    if (definition.grouping) {
         // Currently only time dimensions are supported with parameters
-        return timeDimension(timeDimensionParameters(definition));
+        return Object.assign({ type: 'timeDimension' }, timeDimension(timeDimensionParameters(definition)));
     } else {
-        return `"${definition.column}"`;
+        return { sql: `"${definition.column}"` };
     }
 };
 
-const dimensionNames = (ctx, table) => {
+const dimensionNamesAndExpressions = (ctx) => {
     let dimensions = aggregateDimensions(ctx);
-    return sep(Object.keys(dimensions).map(dimensionName => {
+    return Object.keys(dimensions).map(dimensionName => {
+        const dimension = adaptDimensionDefinition(dimensions[dimensionName]);
+        const expression = dimensionExpression(dimension);
+        return [dimensionName, expression];
+    });
+};
+
+const dimensionNames = (ctx, table) => {
+    return sep(dimensionNamesAndExpressions(ctx).map(([dimensionName, _]) => {
         return table ? `${table}."${dimensionName}"` : `"${dimensionName}"`;
     }));
 };
 
 const dimensionDefs = ctx => {
-    let dimensions = aggregateDimensions(ctx);
-    return sep(Object.keys(dimensions).map(dimensionName => {
-        const dimension = adaptDimensionDefinition(dimensions[dimensionName]);
-        const expression = dimensionExpression(dimension);
-        return `${expression} AS "${dimensionName}"`;
-    }));
+    return sep(dimensionNamesAndExpressions(ctx).map(([dimensionName, expression]) => `${expression.sql} AS "${dimensionName}"`));
 };
 
 const aggregateFilters = ctx => ctx.filters || {};
diff --git a/lib/cartodb/models/aggregation/time-dimension.js b/lib/cartodb/models/aggregation/time-dimension.js
index 1a1909ed..9e897825 100644
--- a/lib/cartodb/models/aggregation/time-dimension.js
+++ b/lib/cartodb/models/aggregation/time-dimension.js
@@ -24,12 +24,7 @@ function timeExpression(t, tz) {
    return t;
 }
 
-// Epoch should be an ISO timestamp literal without time zone
-// (it is interpreted as in the defined timzezone for the input time)
-// It can be partial, e.g. 'YYYY', 'YYYY-MM', 'YYYY-MM-DDTHH', etc.
-// Defaults are applied: YYYY=0001, MM=01, DD=01, HH=00, MM=00, S=00
-// It returns a timestamp without time zone
-function epochExpression(epoch) {
+function epochWithDefaults(epoch) {
     /* jshint maxcomplexity:8 */ // goddammit linter, I like this as is!!
     const format = /^(\d\d\d\d)(?:\-?(\d\d)(?:\-?(\d\d)(?:[T\s]?(\d\d)(?:(\d\d)(?:\:(\d\d))?)?)?)?)?$/;
     const match  = (epoch || '').match(format) || [];
@@ -39,7 +34,15 @@ function epochExpression(epoch) {
     const hour   = match[4] || '00';
     const minute = match[5] || '00';
     const second = match[6] || '00';
-    epoch = `${year}-${month}-${day}T${hour}:${minute}:${second}`;
+    return `${year}-${month}-${day}T${hour}:${minute}:${second}`;
+}
+
+// Epoch should be an ISO timestamp literal without time zone
+// (it is interpreted as in the defined timzezone for the input time)
+// It can be partial, e.g. 'YYYY', 'YYYY-MM', 'YYYY-MM-DDTHH', etc.
+// Defaults are applied: YYYY=0001, MM=01, DD=01, HH=00, MM=00, S=00
+// It returns a timestamp without time zone
+function epochExpression(epoch) {
     return `TIMESTAMP '${epoch}'`;
  }
 
@@ -182,43 +185,27 @@ function validateParameters(params, checker) {
     if (missingParams.length) {
         errors.push(`Missing parameters: ${missingParams.join(', ')}`);
     }
-    errors.push(...checker(params));
+    const params_errors = checker(params);
+    errors.push(...params_errors.errors);
     if (errors.length) {
         throw new Error(`Invalid time dimension:\n${errors.join("\n")}`);
     }
+    return params_errors.params;
 }
 
 const VALID_CYCLIC_GROUPINGS = Object.keys(cyclicParts);
 const VALID_SERIAL_GROUPINGS = Object.keys(serialParts);
 const VALID_ISO_GROUPINGS = Object.keys(isoParts);
 
-const MONTH_GROUPING = {
-  3: 'quarterOfYear',
-  6: 'semesterOfYear',
-  4: 'trimesterOfYear'
-};
-
 function cyclicCheckParams(params) {
     const errors = [];
     if (!VALID_CYCLIC_GROUPINGS.includes(params.grouping)) {
         errors.push(`Invalid grouping "${params.grouping}"`);
-    } else {
-        if (params.count && params.count > 1) {
-            let fixed = false;
-            if (params.grouping === 'monthOfYear') {
-                 const grouping = MONTH_GROUPING[params.count];
-                 if (grouping) {
-                     params.grouping = grouping;
-                     params.count = 1;
-                     fixed = true;
-                 }
-            }
-            if (!fixed) {
-                errors.push(`Invalid count ${params.count} for cyclic ${params.grouping}`);
-            }
-        }
     }
-    return errors;
+    if (params.count && params.count > 1) {
+        errors.push(`Count ${params.count} not supported for cyclic ${params.grouping}`);
+    }
+    return { errors: errors, params: params };
 }
 
 function serialCheckParams(params) {
@@ -226,7 +213,7 @@ function serialCheckParams(params) {
     if (!VALID_SERIAL_GROUPINGS.includes(params.grouping)) {
         errors.push(`Invalid grouping "${params.grouping}"`);
     }
-    return errors;
+    return { errors: errors, params: Object.assign({}, params, { starting: epochWithDefaults(params.starting) }) };
 }
 
 function isoCheckParams(params) {
@@ -237,7 +224,7 @@ function isoCheckParams(params) {
     if (params.starting) {
         errors.push("Parameter 'starting' not supported for ISO format");
     }
-    return errors;
+    return { errors: errors, params: params };
 }
 
 const CLASSIFIERS = {
@@ -271,8 +258,8 @@ function classifierFor(params) {
 
 function classificationSql(params) {
     const classifier = classifierFor(params);
-    validateParameters(params, classifier.checkParams);
-    return classifier.sqlExpr(params);
+    params = validateParameters(params, classifier.checkParams);
+    return { sql: classifier.sqlExpr(params), effectiveParams: params };
 }
 
-module.exports = classificationSql;
\ No newline at end of file
+module.exports = classificationSql;

From c0febf2fd1a673c60891fa562ccf87605ed82a4e Mon Sep 17 00:00:00 2001
From: Javier Goizueta <jgoizueta@gmail.com>
Date: Fri, 5 Oct 2018 20:08:40 +0200
Subject: [PATCH 11/22] Rename time dimension parameters

---
 .../models/aggregation/aggregation-query.js   | 11 +++---
 .../models/aggregation/time-dimension.js      | 36 +++++++++----------
 2 files changed, 24 insertions(+), 23 deletions(-)

diff --git a/lib/cartodb/models/aggregation/aggregation-query.js b/lib/cartodb/models/aggregation/aggregation-query.js
index 8f8d2de8..fdd499f7 100644
--- a/lib/cartodb/models/aggregation/aggregation-query.js
+++ b/lib/cartodb/models/aggregation/aggregation-query.js
@@ -135,12 +135,13 @@ const aggregateDimensions = ctx => ctx.dimensions || {};
 
 const timeDimensionParameters = definition => {
     // definition.column should correspond to a wrapped date column
+    const group = definition.group || {};
     return {
         time: `to_timestamp("${definition.column}")`,
-        timezone: definition.timezone || 'utc',
-        grouping: definition.grouping,
-        count: definition.count || 1,
-        starting: definition.starting,
+        timezone: group.timezone || 'utc',
+        units: group.units,
+        count: group.count || 1,
+        starting: group.starting,
         format: definition.format
     };
 };
@@ -154,7 +155,7 @@ const adaptDimensionDefinition = definition => {
 };
 
 const dimensionExpression = definition => {
-    if (definition.grouping) {
+    if (definition.group) {
         // Currently only time dimensions are supported with parameters
         return Object.assign({ type: 'timeDimension' }, timeDimension(timeDimensionParameters(definition)));
     } else {
diff --git a/lib/cartodb/models/aggregation/time-dimension.js b/lib/cartodb/models/aggregation/time-dimension.js
index 9e897825..27f6de0c 100644
--- a/lib/cartodb/models/aggregation/time-dimension.js
+++ b/lib/cartodb/models/aggregation/time-dimension.js
@@ -111,7 +111,7 @@ const serialParts = {
 };
 
 function serialSqlExpr(params) {
-    const { sql, zeroBased } = serialParts[params.grouping];
+    const { sql, zeroBased } = serialParts[params.units];
     const column = timeExpression(params.time, params.timezone);
     const epoch  = epochExpression(params.starting);
     const serial = sql.replace(/\$t/g, column).replace(/\$epoch/g, epoch);
@@ -150,7 +150,7 @@ function isoSqlExpr(params) {
         // TODO: it would be sensible to return the ISO of the first unit in the period
         throw new Error('Multiple time units not supported for ISO format');
     }
-    return isoParts[params.grouping].replace(/\$t/g, column);
+    return isoParts[params.units].replace(/\$t/g, column);
 }
 
 const cyclicParts = {
@@ -168,11 +168,11 @@ const cyclicParts = {
 
 function cyclicSqlExpr(params) {
     const column = timeExpression(params.time, params.timezone);
-    return cyclicParts[params.grouping].replace(/\$t/g, column);
+    return cyclicParts[params.units].replace(/\$t/g, column);
 }
 
-const ACCEPTED_PARAMETERS = ['time', 'grouping', 'timezone', 'count', 'starting', 'format'];
-const REQUIRED_PARAMETERS = ['time', 'grouping'];
+const ACCEPTED_PARAMETERS = ['time', 'units', 'timezone', 'count', 'starting', 'format'];
+const REQUIRED_PARAMETERS = ['time', 'units'];
 
 function validateParameters(params, checker) {
     const errors = [];
@@ -193,33 +193,33 @@ function validateParameters(params, checker) {
     return params_errors.params;
 }
 
-const VALID_CYCLIC_GROUPINGS = Object.keys(cyclicParts);
-const VALID_SERIAL_GROUPINGS = Object.keys(serialParts);
-const VALID_ISO_GROUPINGS = Object.keys(isoParts);
+const VALID_CYCLIC_UNITS = Object.keys(cyclicParts);
+const VALID_SERIAL_UNITS = Object.keys(serialParts);
+const VALID_ISO_UNITS = Object.keys(isoParts);
 
 function cyclicCheckParams(params) {
     const errors = [];
-    if (!VALID_CYCLIC_GROUPINGS.includes(params.grouping)) {
-        errors.push(`Invalid grouping "${params.grouping}"`);
+    if (!VALID_CYCLIC_UNITS.includes(params.units)) {
+        errors.push(`Invalid units "${params.units}"`);
     }
     if (params.count && params.count > 1) {
-        errors.push(`Count ${params.count} not supported for cyclic ${params.grouping}`);
+        errors.push(`Count ${params.count} not supported for cyclic ${params.units}`);
     }
     return { errors: errors, params: params };
 }
 
 function serialCheckParams(params) {
     const errors = [];
-    if (!VALID_SERIAL_GROUPINGS.includes(params.grouping)) {
-        errors.push(`Invalid grouping "${params.grouping}"`);
+    if (!VALID_SERIAL_UNITS.includes(params.units)) {
+        errors.push(`Invalid grouping units "${params.units}"`);
     }
     return { errors: errors, params: Object.assign({}, params, { starting: epochWithDefaults(params.starting) }) };
 }
 
 function isoCheckParams(params) {
     const errors = [];
-    if (!VALID_ISO_GROUPINGS.includes(params.grouping)) {
-        errors.push(`Invalid grouping "${params.grouping}"`);
+    if (!VALID_ISO_UNITS.includes(params.units)) {
+        errors.push(`Invalid units "${params.units}"`);
     }
     if (params.starting) {
         errors.push("Parameter 'starting' not supported for ISO format");
@@ -242,13 +242,13 @@ const CLASSIFIERS = {
     }
 };
 
-function isCyclic(groupBy) {
-    return VALID_CYCLIC_GROUPINGS.includes(groupBy);
+function isCyclic(units) {
+    return VALID_CYCLIC_UNITS.includes(units);
 }
 
 function classifierFor(params) {
     let classifier = 'serial';
-    if (params.grouping && isCyclic(params.grouping)) {
+    if (params.units && isCyclic(params.units)) {
         classifier = 'cyclic';
     } else if (params.format === 'iso') {
         classifier = 'iso';

From 996d7fc90d55b6774abca924c058b44dd1f8ca6a Mon Sep 17 00:00:00 2001
From: Javier Goizueta <jgoizueta@gmail.com>
Date: Sat, 6 Oct 2018 18:26:43 +0200
Subject: [PATCH 12/22] Lint fixes

---
 lib/cartodb/backends/layer-stats/mapnik-layer-stats.js | 5 +++--
 lib/cartodb/models/aggregation/aggregation-query.js    | 7 +++++--
 lib/cartodb/models/aggregation/time-dimension.js       | 2 +-
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js b/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js
index fb62d532..4888d645 100644
--- a/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js
+++ b/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js
@@ -204,10 +204,11 @@ function _columnStats(ctx, columns, dimensions) {
                 Object.keys(dimensionsInfo).forEach(name => {
                     // Temporalily place dimensions info in stats.columns.__dimensions
                     stats.__dimensions = stats.__dimensions || {};
-                    stats.__dimensions[name] = stats.__dimensions[name] || Object.assign({}, dimensionsStats[name]);
+                    stats.__dimensions[name] = stats.__dimensions[name] || Object.assign({}, dimensionsStats[name]);
                     let type = null;
                     columnAggregations(dimensionsInfo[name]).forEach(fn => {
-                        type = type || fieldTypeSafe(ctx.dbConnection, res.fields.find(f => f.name === `${name}_${fn}`));
+                        type = type ||
+                            fieldTypeSafe(ctx.dbConnection, res.fields.find(f => f.name === `${name}_${fn}`));
                         stats.__dimensions[name][fn] = res.rows[0][`${name}_${fn}`];
                     });
                     stats.__dimensions[name].type = type;
diff --git a/lib/cartodb/models/aggregation/aggregation-query.js b/lib/cartodb/models/aggregation/aggregation-query.js
index fdd499f7..54a8e280 100644
--- a/lib/cartodb/models/aggregation/aggregation-query.js
+++ b/lib/cartodb/models/aggregation/aggregation-query.js
@@ -173,13 +173,16 @@ const dimensionNamesAndExpressions = (ctx) => {
 };
 
 const dimensionNames = (ctx, table) => {
-    return sep(dimensionNamesAndExpressions(ctx).map(([dimensionName, _]) => {
+    return sep(dimensionNamesAndExpressions(ctx).map(([dimensionName]) => {
         return table ? `${table}."${dimensionName}"` : `"${dimensionName}"`;
     }));
 };
 
 const dimensionDefs = ctx => {
-    return sep(dimensionNamesAndExpressions(ctx).map(([dimensionName, expression]) => `${expression.sql} AS "${dimensionName}"`));
+    return sep(
+        dimensionNamesAndExpressions(ctx)
+        .map(([dimensionName, expression]) => `${expression.sql} AS "${dimensionName}"`)
+    );
 };
 
 const aggregateFilters = ctx => ctx.filters || {};
diff --git a/lib/cartodb/models/aggregation/time-dimension.js b/lib/cartodb/models/aggregation/time-dimension.js
index 27f6de0c..6d1fe635 100644
--- a/lib/cartodb/models/aggregation/time-dimension.js
+++ b/lib/cartodb/models/aggregation/time-dimension.js
@@ -25,7 +25,7 @@ function timeExpression(t, tz) {
 }
 
 function epochWithDefaults(epoch) {
-    /* jshint maxcomplexity:8 */ // goddammit linter, I like this as is!!
+    /* jshint maxcomplexity:9 */ // goddammit linter, I like this as is!!
     const format = /^(\d\d\d\d)(?:\-?(\d\d)(?:\-?(\d\d)(?:[T\s]?(\d\d)(?:(\d\d)(?:\:(\d\d))?)?)?)?)?$/;
     const match  = (epoch || '').match(format) || [];
     const year   = match[1] || '0001';

From 10baf43ede66538525cf63bff7ee2a37b1fdff93 Mon Sep 17 00:00:00 2001
From: Javier Goizueta <jgoizueta@gmail.com>
Date: Sun, 7 Oct 2018 00:28:53 +0200
Subject: [PATCH 13/22] Fix dimension metadata bug

---
 lib/cartodb/backends/layer-stats/mapnik-layer-stats.js | 2 +-
 1 file changed, 1 insertion(+), 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 4888d645..e54598bb 100644
--- a/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js
+++ b/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js
@@ -78,7 +78,7 @@ function _geometryType(ctx) {
 }
 
 function _columns(ctx) {
-    if (ctx.metaOptions.columns || ctx.metaOptions.columnStats) {
+    if (ctx.metaOptions.columns || ctx.metaOptions.columnStats || ctx.metaOptions.dimensions) {
         // note: post-aggregation columns are in layer.options.columns when aggregation is present
         return queryUtils.queryPromise(ctx.dbConnection, _getSQL(ctx, sql => queryUtils.getQueryLimited(sql, 0)))
             .then(res => formatResultFields(ctx.dbConnection, res.fields));

From 2f59919f84d15aa0f61065a6160c389c48b9fa75 Mon Sep 17 00:00:00 2001
From: Javier Goizueta <jgoizueta@gmail.com>
Date: Sun, 7 Oct 2018 00:29:12 +0200
Subject: [PATCH 14/22] Dimension metadata test

---
 test/acceptance/aggregation.js | 50 ++++++++++++++++++++++++++++++++--
 1 file changed, 47 insertions(+), 3 deletions(-)

diff --git a/test/acceptance/aggregation.js b/test/acceptance/aggregation.js
index 427983e4..d3ccb34a 100644
--- a/test/acceptance/aggregation.js
+++ b/test/acceptance/aggregation.js
@@ -878,8 +878,7 @@ describe('aggregation', function () {
                 });
             });
 
-            it('time dimensions',
-            function (done) {
+            it('time dimensions', function (done) {
                 this.mapConfig = createVectorMapConfig([
                     {
                         type: 'cartodb',
@@ -891,7 +890,9 @@ describe('aggregation', function () {
                                 dimensions: {
                                     dow: {
                                         column: 'date',
-                                        group_by: 'dayOfWeek'
+                                        group: {
+                                            units: 'dayOfWeek'
+                                        }
                                     }
                                 }
                             }
@@ -916,7 +917,50 @@ describe('aggregation', function () {
                 });
             });
 
+            // TODO: cyclic with timezone, serial, serial with count/starting/timezone, serial iso / with timezone
 
+            it('time dimensions stats', function (done) {
+                this.mapConfig = createVectorMapConfig([
+                    {
+                        type: 'cartodb',
+                        options: {
+                            sql: POINTS_SQL_TIMESTAMP_1,
+                            dates_as_numbers: true,
+                            aggregation: {
+                                threshold: 1,
+                                dimensions: {
+                                    dow: {
+                                        column: 'date',
+                                        group: {
+                                            units: 'dayOfWeek'
+                                        }
+                                    }
+                                }
+                            },
+                            metadata: {
+                                dimensions: true
+                            }
+                        }
+                    }
+                ]);
+
+                this.testClient = new TestClient(this.mapConfig);
+                this.testClient.getLayergroup(function(err, layergroup) {
+                    assert.ifError(err);
+                    const expectedDimensions = {
+                        dow:
+                        { params:
+                           { time: 'to_timestamp("date")',
+                             timezone: 'utc',
+                             units: 'dayOfWeek',
+                             count: 1 },
+                          min: 4,
+                          max: 7,
+                          type: 'number' }
+                    };
+                    assert.deepEqual(layergroup.metadata.layers[0].meta.stats.dimensions, expectedDimensions);
+                });
+            });
 
             ['centroid', 'point-sample', 'point-grid'].forEach(placement => {
                 it(`dimensions should work for ${placement} placement`, function(done) {

From 0e85aa56da2c8394bc7a514884f5d441bc97dd39 Mon Sep 17 00:00:00 2001
From: Javier Goizueta <jgoizueta@gmail.com>
Date: Sun, 7 Oct 2018 11:35:28 +0200
Subject: [PATCH 15/22] Fix test

---
 test/acceptance/aggregation.js | 1 +
 1 file changed, 1 insertion(+)

diff --git a/test/acceptance/aggregation.js b/test/acceptance/aggregation.js
index d3ccb34a..7b1ae7b3 100644
--- a/test/acceptance/aggregation.js
+++ b/test/acceptance/aggregation.js
@@ -959,6 +959,7 @@ describe('aggregation', function () {
                           type: 'number' }
                     };
                     assert.deepEqual(layergroup.metadata.layers[0].meta.stats.dimensions, expectedDimensions);
+                    done();
                 });
             });
 

From 9ed39f149bbeab4f845b6fc959fa0b319be582bf Mon Sep 17 00:00:00 2001
From: Javier Goizueta <jgoizueta@gmail.com>
Date: Sun, 7 Oct 2018 22:46:02 +0200
Subject: [PATCH 16/22] Time dimension tests

---
 test/acceptance/aggregation.js | 373 ++++++++++++++++++++++++++++++++-
 1 file changed, 372 insertions(+), 1 deletion(-)

diff --git a/test/acceptance/aggregation.js b/test/acceptance/aggregation.js
index 7b1ae7b3..ba9083e7 100644
--- a/test/acceptance/aggregation.js
+++ b/test/acceptance/aggregation.js
@@ -16,6 +16,36 @@ if (process.env.POSTGIS_VERSION >= '20400') {
     });
 }
 
+// Generate points with values and times.
+// The point location is spanned over a given length, by default it is 0 so
+// all points have the same location, which can be used to test aggregation dimensions
+// the default point is in tile
+function pointsWithTimeSQL(n, startTime, endTime, span = 0, x0 = 0.1, y0 = 0.1) {
+    return `
+        WITH params AS (
+        SELECT
+        '${startTime}'::timestamp with time zone AS min_t,
+        '${endTime}'::timestamp with time zone AS max_t,
+        ${x0} AS x0, ${y0} AS y0,
+        ${span} AS length,
+        ${n} AS n
+        ),
+        positions AS (
+        SELECT
+            step::float8/n AS s,
+            x0 + (step::float8/n - 0.5)*length AS x, y0 AS y
+        FROM params, generate_series(1, n) AS step
+        )
+        SELECT
+        row_number() over () AS cartodb_id,
+        n*10 AS value,
+        min_t + (max_t - min_t)*s AS date,
+        ST_SetSRID(ST_MakePoint(x, y), 4326) AS the_geom,
+        ST_Transform(ST_SetSRID(ST_MakePoint(x, y), 4326), 3857) AS the_geom_webmercator
+        FROM params, positions
+    `;
+}
+
 describe('aggregation', function () {
 
     const POINTS_SQL_1 = `
@@ -917,7 +947,232 @@ describe('aggregation', function () {
                 });
             });
 
-            // TODO: cyclic with timezone, serial, serial with count/starting/timezone, serial iso / with timezone
+            it('aggregation dimensions only used if present', function (done) {
+                this.mapConfig = createVectorMapConfig([
+                    {
+                        type: 'cartodb',
+                        options: {
+                            sql: pointsWithTimeSQL(50, '2018-01-01T00:00:00+00', '2018-12-31T23:59:59+00', 0),
+                            dates_as_numbers: true,
+                            aggregation: {
+                                threshold: 1,
+                            }
+                        }
+                    }
+                ]);
+
+                this.testClient = new TestClient(this.mapConfig);
+                const options = {
+                    format: 'mvt'
+                };
+                this.testClient.getTile(0, 0, 0, options, (err, res, tile) => {
+                    if (err) {
+                        return done(err);
+                    }
+                    const tileJSON = tile.toJSON();
+                    assert.equal(tileJSON[0].features.length, 1);
+                    done();
+                });
+            });
+
+            it('aggregation dimension month used', function (done) {
+                this.mapConfig = createVectorMapConfig([
+                    {
+                        type: 'cartodb',
+                        options: {
+                            sql: pointsWithTimeSQL(50, '2018-01-01T00:00:00+00', '2018-12-31T23:59:59+00', 0),
+                            dates_as_numbers: true,
+                            aggregation: {
+                                threshold: 1,
+                                dimensions: {
+                                    month: {
+                                        column: 'date',
+                                        group: {
+                                            units: 'month'
+                                        }
+                                    }
+                                }
+
+                            },
+                        }
+                    }
+                ]);
+
+                this.testClient = new TestClient(this.mapConfig);
+                const options = {
+                    format: 'mvt'
+                };
+                this.testClient.getTile(0, 0, 0, options, (err, res, tile) => {
+                    if (err) {
+                        return done(err);
+                    }
+                    const tileJSON = tile.toJSON();
+                    assert.equal(tileJSON[0].features.length, 12);
+
+                    done();
+                });
+            });
+
+            it('aggregation dimension month with count', function (done) {
+                this.mapConfig = createVectorMapConfig([
+                    {
+                        type: 'cartodb',
+                        options: {
+                            sql: pointsWithTimeSQL(50, '2018-01-01T00:00:00+00', '2018-12-31T23:59:59+00', 0),
+                            dates_as_numbers: true,
+                            aggregation: {
+                                threshold: 1,
+                                dimensions: {
+                                    month: {
+                                        column: 'date',
+                                        group: {
+                                            units: 'month',
+                                            count: 5,
+                                            starting: '2018-01'
+                                        }
+                                    }
+                                }
+
+                            },
+                        }
+                    }
+                ]);
+
+                this.testClient = new TestClient(this.mapConfig);
+                const options = {
+                    format: 'mvt'
+                };
+                this.testClient.getTile(0, 0, 0, options, (err, res, tile) => {
+                    if (err) {
+                        return done(err);
+                    }
+                    const tileJSON = tile.toJSON();
+                    assert.equal(tileJSON[0].features.length, 3);
+                    const resultQuimesters = tileJSON[0].features.map(f => f.properties.month).sort((a, b) => a - b);
+                    assert.deepEqual(resultQuimesters, [1, 2, 3]);
+
+                    done();
+                });
+            });
+
+            it('aggregation dimension month with starting', function (done) {
+                this.mapConfig = createVectorMapConfig([
+                    {
+                        type: 'cartodb',
+                        options: {
+                            sql: pointsWithTimeSQL(50, '2018-01-01T00:00:00+00', '2018-12-31T23:59:59+00', 0),
+                            dates_as_numbers: true,
+                            aggregation: {
+                                threshold: 1,
+                                dimensions: {
+                                    month: {
+                                        column: 'date',
+                                        group: {
+                                            units: 'month',
+                                            starting: '2017-01'
+                                        }
+                                    }
+                                }
+
+                            }
+                        }
+                    }
+                ]);
+
+                this.testClient = new TestClient(this.mapConfig);
+                const options = {
+                    format: 'mvt'
+                };
+                this.testClient.getTile(0, 0, 0, options, (err, res, tile) => {
+                    if (err) {
+                        return done(err);
+                    }
+                    const tileJSON = tile.toJSON();
+                    const resultMonths = tileJSON[0].features.map(f => f.properties.month).sort((a, b) => a - b);
+                    assert.deepEqual(resultMonths, [13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24]);
+
+                    done();
+                });
+            });
+
+            it('aggregation dimension month by default UTC', function (done) {
+                this.mapConfig = createVectorMapConfig([
+                    {
+                        type: 'cartodb',
+                        options: {
+                            sql: pointsWithTimeSQL(50, '2018-01-01T00:00:00+00', '2018-01-31T23:59:59+00', 0),
+                            dates_as_numbers: true,
+                            aggregation: {
+                                threshold: 1,
+                                dimensions: {
+                                    dow: {
+                                        column: 'date',
+                                        group: {
+                                            units: 'month',
+                                            timezone: '+00'
+                                        }
+                                    }
+                                }
+
+                            }
+                        }
+                    }
+                ]);
+
+                this.testClient = new TestClient(this.mapConfig);
+                const options = {
+                    format: 'mvt'
+                };
+                this.testClient.getTile(0, 0, 0, options, (err, res, tile) => {
+                    if (err) {
+                        return done(err);
+                    }
+                    const tileJSON = tile.toJSON();
+                    // In UTC all times are in the same month 2018-01
+                    assert.equal(tileJSON[0].features.length, 1);
+
+                    done();
+                });
+            });
+
+            it('aggregation dimension month with timezone', function (done) {
+                this.mapConfig = createVectorMapConfig([
+                    {
+                        type: 'cartodb',
+                        options: {
+                            sql: pointsWithTimeSQL(50, '2018-01-01T00:00:00+00', '2018-01-31T23:59:59+00', 0),
+                            dates_as_numbers: true,
+                            aggregation: {
+                                threshold: 1,
+                                dimensions: {
+                                    dow: {
+                                        column: 'date',
+                                        group: {
+                                            units: 'month',
+                                            timezone: '+7200'
+                                        }
+                                    }
+                                }
+
+                            }
+                        }
+                    }
+                ]);
+
+                this.testClient = new TestClient(this.mapConfig);
+                const options = {
+                    format: 'mvt'
+                };
+                this.testClient.getTile(0, 0, 0, options, (err, res, tile) => {
+                    if (err) {
+                        return done(err);
+                    }
+                    const tileJSON = tile.toJSON();
+                    // In UTC+2 some times are in a different month
+                    assert.equal(tileJSON[0].features.length, 2);
+                    done();
+                });
+            });
 
             it('time dimensions stats', function (done) {
                 this.mapConfig = createVectorMapConfig([
@@ -963,6 +1218,122 @@ describe('aggregation', function () {
                 });
             });
 
+            it('no time dimensions stats by default', function (done) {
+                this.mapConfig = createVectorMapConfig([
+                    {
+                        type: 'cartodb',
+                        options: {
+                            sql: POINTS_SQL_TIMESTAMP_1,
+                            dates_as_numbers: true,
+                            aggregation: {
+                                threshold: 1,
+                                dimensions: {
+                                    dow: {
+                                        column: 'date',
+                                        group: {
+                                            units: 'dayOfWeek'
+                                        }
+                                    }
+                                }
+                            }
+                        }
+                    }
+                ]);
+
+                this.testClient = new TestClient(this.mapConfig);
+                this.testClient.getLayergroup(function(err, layergroup) {
+                    assert.ifError(err);
+                    assert(!layergroup.metadata.layers[0].meta.stats.dimensions);
+                    done();
+                });
+            });
+
+            it('aggregation dimension month iso format', function (done) {
+                this.mapConfig = createVectorMapConfig([
+                    {
+                        type: 'cartodb',
+                        options: {
+                            sql: pointsWithTimeSQL(50, '2018-01-01T00:00:00+00', '2018-12-31T23:59:59+00', 0),
+                            dates_as_numbers: true,
+                            aggregation: {
+                                threshold: 1,
+                                dimensions: {
+                                    month: {
+                                        column: 'date',
+                                        group: {
+                                            units: 'month',
+                                        },
+                                        format: 'iso'
+                                    }
+                                }
+
+                            }
+                        }
+                    }
+                ]);
+
+                this.testClient = new TestClient(this.mapConfig);
+                const options = {
+                    format: 'mvt'
+                };
+                this.testClient.getTile(0, 0, 0, options, (err, res, tile) => {
+                    if (err) {
+                        return done(err);
+                    }
+                    const tileJSON = tile.toJSON();
+                    const resultMonths = tileJSON[0].features.map(f => f.properties.month).sort();
+                    assert.deepEqual(resultMonths, [
+                        '2018-01', '2018-02', '2018-03', '2018-04', '2018-05','2018-06',
+                        '2018-07', '2018-08', '2018-09', '2018-10', '2018-11', '2018-12'
+                    ]);
+                    done();
+                });
+            });
+
+            it('aggregation dimension month iso format with timezone', function (done) {
+                this.mapConfig = createVectorMapConfig([
+                    {
+                        type: 'cartodb',
+                        options: {
+                            sql: pointsWithTimeSQL(50, '2018-01-01T00:00:00+00', '2018-12-31T23:59:59+00', 0),
+                            dates_as_numbers: true,
+                            aggregation: {
+                                threshold: 1,
+                                dimensions: {
+                                    month: {
+                                        column: 'date',
+                                        group: {
+                                            units: 'month',
+                                            timezone: '+7200'
+                                        },
+                                        format: 'iso',
+                                    }
+                                }
+
+                            }
+                        }
+                    }
+                ]);
+
+                this.testClient = new TestClient(this.mapConfig);
+                const options = {
+                    format: 'mvt'
+                };
+                this.testClient.getTile(0, 0, 0, options, (err, res, tile) => {
+                    if (err) {
+                        return done(err);
+                    }
+                    const tileJSON = tile.toJSON();
+                    const resultMonths = tileJSON[0].features.map(f => f.properties.month).sort();
+                    assert.deepEqual(resultMonths, [
+                        '2018-01', '2018-02', '2018-03', '2018-04', '2018-05', '2018-06',
+                        '2018-07', '2018-08', '2018-09', '2018-10', '2018-11', '2018-12',
+                        '2019-01'
+                    ]);
+                    done();
+                });
+            });
+
             ['centroid', 'point-sample', 'point-grid'].forEach(placement => {
                 it(`dimensions should work for ${placement} placement`, function(done) {
                     this.mapConfig = createVectorMapConfig([

From a4dfc09c719b3f78e08ed20d62c93f200a3394e4 Mon Sep 17 00:00:00 2001
From: Javier Goizueta <jgoizueta@gmail.com>
Date: Sun, 7 Oct 2018 23:12:41 +0200
Subject: [PATCH 17/22] Aggregation dimensions documentation

---
 NEWS.md             |  5 +++++
 docs/aggregation.md | 55 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+)

diff --git a/NEWS.md b/NEWS.md
index 813d9595..985d0f00 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -1,5 +1,10 @@
 # Changelog
 
+## 6.5.0
+
+New features
+- Aggregation time dimensions
+
 ## 6.4.0
 Released 2018-mm-dd
 
diff --git a/docs/aggregation.md b/docs/aggregation.md
index 6920776f..d20359cb 100644
--- a/docs/aggregation.md
+++ b/docs/aggregation.md
@@ -134,6 +134,61 @@ of the original dataset applying three different aggregate functions.
 
 > Note that you can use the original column names as names of the result, but all the result column names must be unique.  In particular, the names `cartodb_id`, `the_geom`, `the_geom_webmercator` and `_cdb_feature_count` cannot be used for aggregated columns, as they correspond to columns always present in the result.
 
+### `dimensions`
+
+By default aggregated data is grouped only by the the spatial discretization into grid cells.
+The `dimensions` parameters permits to define additional aggregation dimensions. Currently only date (time, timestamp) columns can be used as dimensions. Just like the spatial position is discretized using a grid, for every
+additional dimension, a grouping into discrete periods of time must be defined.
+
+```json
+{
+    "dimensions": {
+        "time_in_days": { // Name of the resulting dimension column
+            "column": "the_time", // column discretized for the dimension
+            "group": { // defines how to group the values of the column
+                "units": "day", // grouping units
+                "count": 1, // optional, number of grouping units per period, 1 by default
+                "timezone": "Europe/Madrid", // optional (by default 'utc' == '+0000')
+                "starting": "2018-01-01" // by default '0001-01-01T00:00:00', periods counted from this time
+            },
+            "format": "number" // optional, "iso" indicates ISO 8601 format
+        }
+    }
+}
+```
+
+The `column` used as an aggregation dimension must be of type date, time or timestamp (preferably with time zone), and `dates_as_numbers` must be active.
+
+There are two kind of `units`:
+
+* serial: `second`, `minute`, `hour`, `day`, `year`, `week`, `quarter`, `trimester`, `semester`, `decade`, `century`, `millenium`.
+* cyclic: `dayOfWeek` (day of the week), `dayOfMonth` (day of the month), `dayOfYear` (day of the year), `hourofDay` (hour of the day), `monthOfYear` (month of the year), `quarterOfYear`, `semesterOfYear`, `trimesterOfYear`, `weekOfDay`, `minuteOfHour`.
+
+Serial units correspond to time series counted from a starting time; for serial units, unless `format` is ISO, a `count` greater than one can be specified, so that the grouping period could be 5 days, 2 monthos or 10 minutes.
+
+Cyclic units repeat over a second period given by another unit (day of the month, day of the week...). For example month of the year, which will produce values from 1 to 12, will group the values corresponding of januray of any year to 1. The numbers follow conventional use with units smaller than the day starting at value 0 and the rest at 1, so we have for example months 1 to 12, day of the month 1 to 31, hour of the day 0 to 23, etc. For days of the week the ISO standard convention is followed with 1=monday to 7=sunday; weeks also follow the ISO convention with the first week of a year containing the first thursday.
+
+In numeric form (the default `format: 'number'`), serial units count the grouping periods since the `starting` time. The starting time is by default the first proleptic Gregorian year CE (0001-01-01T00:00:00), so that if the periods are years, the numeric values are as expected.
+
+The `timezone` is used to delimit the start of days when grouping a time. A fixed offset from UTC can be given as a signed number of seconds, or [IANA](https://www.iana.org/time-zones) timezone [names](https://en.wikipedia.org/wiki/List_of_tz_database_time_zones) such as `Europe/Madrid` can be used, which can define non-fixed offsets due to daylight saving time changes.
+
+The starting time is given as a string in [ISO format](https://en.wikipedia.org/wiki/ISO_8601). It is interpreted in the same timezone used for grouping, and should not include a timezone offset. It can be defined partially, e.g. `2018-01` for
+`2018-01-01T00:00:00`.
+
+In ISO form, serial units are structured in text form always counted in the conventional form year-month-day etc.
+This format is based on ISO 8601 (YYYY-MM, YYYY-MM-DD, YYYY-MM-DDTHH, etc.), with some additional convention:
+* Weeks are represented as; 2018W03 (third week of 2018:, 2018-01-15 to 2018-01-21)
+* Quarters as 2018Q2 (second quarter of 2018, i.e. 2018-04 to 2018-06)
+* Semesters as 2018S2 (second semester of 2018, i.e. 2018-07 to 2018-12)
+* Trimesters as 2018t2 (second trimester of 2018, i.e. 2018-05 to 2018-08)
+* Decades as D201 (decade 201, i.e. 2010 to 2019)
+* Centuries as C21 (21st century, ie. 2001 to 2100)
+* Millenniums as M3 (3rd millinnium: 2001 to 3000)
+
+#### Limitations:
+* The iso text format does not admit `starting` or `count` parameters
+* Cyclic units (day of the week, etc.) don't admit `count` or `starting` either.
+
 ### `resolution`
 
 Defines the cell-size of the spatial aggregation grid. This is equivalent to the [CartoCSS `-torque-resolution`](https://carto.com/docs/carto-engine/cartocss/properties-for-torque/#-torque-resolution-float) property of Torque maps.

From d4bd706fe2b20fb4c194f391190a20bd862b59e5 Mon Sep 17 00:00:00 2001
From: Javier Goizueta <jgoizueta@gmail.com>
Date: Mon, 8 Oct 2018 19:16:32 +0200
Subject: [PATCH 18/22] Clarify some tests

---
 test/acceptance/aggregation.js | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/test/acceptance/aggregation.js b/test/acceptance/aggregation.js
index ba9083e7..095eca51 100644
--- a/test/acceptance/aggregation.js
+++ b/test/acceptance/aggregation.js
@@ -948,11 +948,12 @@ describe('aggregation', function () {
             });
 
             it('aggregation dimensions only used if present', function (done) {
+                const nPoints = 50;
                 this.mapConfig = createVectorMapConfig([
                     {
                         type: 'cartodb',
                         options: {
-                            sql: pointsWithTimeSQL(50, '2018-01-01T00:00:00+00', '2018-12-31T23:59:59+00', 0),
+                            sql: pointsWithTimeSQL(nPoints, '2000-01-01T00:00:00+00', '2019-12-31T23:59:59+00', 0),
                             dates_as_numbers: true,
                             aggregation: {
                                 threshold: 1,
@@ -970,25 +971,28 @@ describe('aggregation', function () {
                         return done(err);
                     }
                     const tileJSON = tile.toJSON();
-                    assert.equal(tileJSON[0].features.length, 1);
+                    // Everything's aggregated into a single feature because the only
+                    // dimension is space and all points are in the same place.
+                    assert.deepEqual(tileJSON[0].features.map(f => f.properties._cdb_feature_count), [nPoints]);
                     done();
                 });
             });
 
-            it('aggregation dimension month used', function (done) {
+            it('aggregation dimension year used', function (done) {
+                const nPoints = 50;
                 this.mapConfig = createVectorMapConfig([
                     {
                         type: 'cartodb',
                         options: {
-                            sql: pointsWithTimeSQL(50, '2018-01-01T00:00:00+00', '2018-12-31T23:59:59+00', 0),
+                            sql: pointsWithTimeSQL(nPoints, '2000-01-01T00:00:00+00', '2019-12-31T23:59:59+00', 0),
                             dates_as_numbers: true,
                             aggregation: {
                                 threshold: 1,
                                 dimensions: {
-                                    month: {
+                                    year: {
                                         column: 'date',
                                         group: {
-                                            units: 'month'
+                                            units: 'year'
                                         }
                                     }
                                 }
@@ -1007,7 +1011,11 @@ describe('aggregation', function () {
                         return done(err);
                     }
                     const tileJSON = tile.toJSON();
-                    assert.equal(tileJSON[0].features.length, 12);
+                    // Now all features have same location, but the year is an additional dimension
+                    // with 20 different values, so we'll have an aggregated feature for each.
+                    const expectedYears = Array.from({length: 20}, (_, k) => 2000 + k); // 2000 to 2019
+                    const resultYears = tileJSON[0].features.map(f => f.properties.year).sort((a, b) => a - b);
+                    assert.deepEqual(resultYears, expectedYears);
 
                     done();
                 });

From 418e0e2aa3cfbb2ed648d6c895c06d7d1eba4566 Mon Sep 17 00:00:00 2001
From: Javier Goizueta <jgoizueta@gmail.com>
Date: Mon, 8 Oct 2018 19:16:50 +0200
Subject: [PATCH 19/22] Documentation corrections

---
 docs/aggregation.md | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/docs/aggregation.md b/docs/aggregation.md
index d20359cb..bac6b3ce 100644
--- a/docs/aggregation.md
+++ b/docs/aggregation.md
@@ -140,7 +140,7 @@ By default aggregated data is grouped only by the the spatial discretization int
 The `dimensions` parameters permits to define additional aggregation dimensions. Currently only date (time, timestamp) columns can be used as dimensions. Just like the spatial position is discretized using a grid, for every
 additional dimension, a grouping into discrete periods of time must be defined.
 
-```json
+```js
 {
     "dimensions": {
         "time_in_days": { // Name of the resulting dimension column
@@ -164,7 +164,7 @@ There are two kind of `units`:
 * serial: `second`, `minute`, `hour`, `day`, `year`, `week`, `quarter`, `trimester`, `semester`, `decade`, `century`, `millenium`.
 * cyclic: `dayOfWeek` (day of the week), `dayOfMonth` (day of the month), `dayOfYear` (day of the year), `hourofDay` (hour of the day), `monthOfYear` (month of the year), `quarterOfYear`, `semesterOfYear`, `trimesterOfYear`, `weekOfDay`, `minuteOfHour`.
 
-Serial units correspond to time series counted from a starting time; for serial units, unless `format` is ISO, a `count` greater than one can be specified, so that the grouping period could be 5 days, 2 monthos or 10 minutes.
+Serial units correspond to time series counted from a starting time; for serial units, unless `format` is ISO, a `count` greater than one can be specified, so that the grouping period could be 5 days, 2 months or 10 minutes.
 
 Cyclic units repeat over a second period given by another unit (day of the month, day of the week...). For example month of the year, which will produce values from 1 to 12, will group the values corresponding of januray of any year to 1. The numbers follow conventional use with units smaller than the day starting at value 0 and the rest at 1, so we have for example months 1 to 12, day of the month 1 to 31, hour of the day 0 to 23, etc. For days of the week the ISO standard convention is followed with 1=monday to 7=sunday; weeks also follow the ISO convention with the first week of a year containing the first thursday.
 

From ee63b247cd2875298aa5c6fa6e1f10beb849f341 Mon Sep 17 00:00:00 2001
From: Javier Goizueta <jgoizueta@gmail.com>
Date: Mon, 8 Oct 2018 19:25:04 +0200
Subject: [PATCH 20/22] Slight refactor

---
 lib/cartodb/backends/layer-stats/mapnik-layer-stats.js | 9 ++++++++-
 1 file changed, 8 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 e54598bb..45bcc58a 100644
--- a/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js
+++ b/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js
@@ -142,11 +142,18 @@ function _sample(ctx, numRows) {
     return Promise.resolve();
 }
 
+function _columnsMetadataRequired(options) {
+    // We need determine the columns of a query
+    // if either column stats or dimension stats are required,
+    // since we'll ultimately use the same query to fetch both
+    return options.columnStats || options.dimensions;
+}
+
 function _columnStats(ctx, columns, dimensions) {
     if (!columns) {
         return Promise.resolve();
     }
-    if (ctx.metaOptions.columnStats || ctx.metaOptions.dimensions) {
+    if (_columnsMetadataRequired(ctx.metaOptions)) {
         let queries = [];
         let aggr = [];
         if (ctx.metaOptions.columnStats) {

From 73b3402d859dd4fc656e50fb48c42a5b02cf0b28 Mon Sep 17 00:00:00 2001
From: Javier Goizueta <jgoizueta@gmail.com>
Date: Tue, 9 Oct 2018 13:24:08 +0200
Subject: [PATCH 21/22] Refactor stats collection

---
 .../layer-stats/mapnik-layer-stats.js         | 29 ++++++++-----------
 1 file changed, 12 insertions(+), 17 deletions(-)

diff --git a/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js b/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js
index 45bcc58a..67d00d90 100644
--- a/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js
+++ b/lib/cartodb/backends/layer-stats/mapnik-layer-stats.js
@@ -2,7 +2,6 @@ const queryUtils = require('../../utils/query-utils');
 const AggregationMapConfig = require('../../models/aggregation/aggregation-mapconfig');
 const aggregationQuery = require('../../models/aggregation/aggregation-query');
 
-
 function MapnikLayerStats () {
     this._types = {
         mapnik: true,
@@ -157,7 +156,7 @@ function _columnStats(ctx, columns, dimensions) {
         let queries = [];
         let aggr = [];
         if (ctx.metaOptions.columnStats) {
-            queries.push(new Promise(resolve => resolve(columns))); // add columns as first result
+            queries.push(new Promise(resolve => resolve({ columns }))); // add columns as first result
             Object.keys(columns).forEach(name => {
                 aggr = aggr.concat(
                     columnAggregations(columns[name])
@@ -175,7 +174,7 @@ function _columnStats(ctx, columns, dimensions) {
                         queryUtils.queryPromise(
                             ctx.dbConnection,
                             _getSQL(ctx, sql => queryUtils.getQueryTopCategories(sql, name, topN, includeNulls))
-                        ).then(res => ({ [name]: { categories: res.rows } }))
+                        ).then(res => ({ columns: { [name]: { categories: res.rows } } }))
                     );
                 }
             });
@@ -201,29 +200,30 @@ function _columnStats(ctx, columns, dimensions) {
                 ctx.dbConnection,
                 _getSQL(ctx, sql => `SELECT ${aggr.join(',')} FROM (${sql}) AS __cdb_query`)
             ).then(res => {
-                let stats = {};
+                let stats = { columns: {}, dimensions: {} };
                 Object.keys(columns).forEach(name => {
-                    stats[name] = {};
+                    stats.columns[name] = {};
                     columnAggregations(columns[name]).forEach(fn => {
-                        stats[name][fn] = res.rows[0][`${name}_${fn}`];
+                        stats.columns[name][fn] = res.rows[0][`${name}_${fn}`];
                     });
                 });
                 Object.keys(dimensionsInfo).forEach(name => {
-                    // Temporalily place dimensions info in stats.columns.__dimensions
-                    stats.__dimensions = stats.__dimensions || {};
-                    stats.__dimensions[name] = stats.__dimensions[name] || Object.assign({}, dimensionsStats[name]);
+                    stats.dimensions[name] = stats.dimensions[name] || Object.assign({}, dimensionsStats[name]);
                     let type = null;
                     columnAggregations(dimensionsInfo[name]).forEach(fn => {
                         type = type ||
                             fieldTypeSafe(ctx.dbConnection, res.fields.find(f => f.name === `${name}_${fn}`));
-                        stats.__dimensions[name][fn] = res.rows[0][`${name}_${fn}`];
+                        stats.dimensions[name][fn] = res.rows[0][`${name}_${fn}`];
                     });
-                    stats.__dimensions[name].type = type;
+                    stats.dimensions[name].type = type;
                 });
                 return stats;
             })
         );
-        return Promise.all(queries).then(results => ({ columns: mergeColumns(results) }));
+        return Promise.all(queries).then(results => ({
+            columns: mergeColumns(results.map(r => r.columns)),
+            dimensions: mergeColumns(results.map( r => r.dimensions))
+        }));
     }
     return Promise.resolve({ columns });
 }
@@ -301,11 +301,6 @@ function (layer, dbConnection, callback) {
         _columns(ctx).then(columns => _columnStats(ctx, columns, dimensions))
     ]).then(results => {
         results = mergeResults(results);
-        const dimensions = results.columns && results.columns.__dimensions;
-        if (dimensions) {
-            delete results.columns.__dimensions;
-            results.dimensions = dimensions;
-        }
         callback(null, results);
     }).catch(error => {
         callback(error);

From 41bd69d050ed4026235838b670b3af8243a617c8 Mon Sep 17 00:00:00 2001
From: Javier Goizueta <jgoizueta@gmail.com>
Date: Tue, 9 Oct 2018 15:17:29 +0200
Subject: [PATCH 22/22] Remove public docs for the time being

We might want to make changes to the API after initial test usage
---
 docs/aggregation.md | 51 ---------------------------------------------
 1 file changed, 51 deletions(-)

diff --git a/docs/aggregation.md b/docs/aggregation.md
index bac6b3ce..506e7c77 100644
--- a/docs/aggregation.md
+++ b/docs/aggregation.md
@@ -134,57 +134,6 @@ of the original dataset applying three different aggregate functions.
 
 > Note that you can use the original column names as names of the result, but all the result column names must be unique.  In particular, the names `cartodb_id`, `the_geom`, `the_geom_webmercator` and `_cdb_feature_count` cannot be used for aggregated columns, as they correspond to columns always present in the result.
 
-### `dimensions`
-
-By default aggregated data is grouped only by the the spatial discretization into grid cells.
-The `dimensions` parameters permits to define additional aggregation dimensions. Currently only date (time, timestamp) columns can be used as dimensions. Just like the spatial position is discretized using a grid, for every
-additional dimension, a grouping into discrete periods of time must be defined.
-
-```js
-{
-    "dimensions": {
-        "time_in_days": { // Name of the resulting dimension column
-            "column": "the_time", // column discretized for the dimension
-            "group": { // defines how to group the values of the column
-                "units": "day", // grouping units
-                "count": 1, // optional, number of grouping units per period, 1 by default
-                "timezone": "Europe/Madrid", // optional (by default 'utc' == '+0000')
-                "starting": "2018-01-01" // by default '0001-01-01T00:00:00', periods counted from this time
-            },
-            "format": "number" // optional, "iso" indicates ISO 8601 format
-        }
-    }
-}
-```
-
-The `column` used as an aggregation dimension must be of type date, time or timestamp (preferably with time zone), and `dates_as_numbers` must be active.
-
-There are two kind of `units`:
-
-* serial: `second`, `minute`, `hour`, `day`, `year`, `week`, `quarter`, `trimester`, `semester`, `decade`, `century`, `millenium`.
-* cyclic: `dayOfWeek` (day of the week), `dayOfMonth` (day of the month), `dayOfYear` (day of the year), `hourofDay` (hour of the day), `monthOfYear` (month of the year), `quarterOfYear`, `semesterOfYear`, `trimesterOfYear`, `weekOfDay`, `minuteOfHour`.
-
-Serial units correspond to time series counted from a starting time; for serial units, unless `format` is ISO, a `count` greater than one can be specified, so that the grouping period could be 5 days, 2 months or 10 minutes.
-
-Cyclic units repeat over a second period given by another unit (day of the month, day of the week...). For example month of the year, which will produce values from 1 to 12, will group the values corresponding of januray of any year to 1. The numbers follow conventional use with units smaller than the day starting at value 0 and the rest at 1, so we have for example months 1 to 12, day of the month 1 to 31, hour of the day 0 to 23, etc. For days of the week the ISO standard convention is followed with 1=monday to 7=sunday; weeks also follow the ISO convention with the first week of a year containing the first thursday.
-
-In numeric form (the default `format: 'number'`), serial units count the grouping periods since the `starting` time. The starting time is by default the first proleptic Gregorian year CE (0001-01-01T00:00:00), so that if the periods are years, the numeric values are as expected.
-
-The `timezone` is used to delimit the start of days when grouping a time. A fixed offset from UTC can be given as a signed number of seconds, or [IANA](https://www.iana.org/time-zones) timezone [names](https://en.wikipedia.org/wiki/List_of_tz_database_time_zones) such as `Europe/Madrid` can be used, which can define non-fixed offsets due to daylight saving time changes.
-
-The starting time is given as a string in [ISO format](https://en.wikipedia.org/wiki/ISO_8601). It is interpreted in the same timezone used for grouping, and should not include a timezone offset. It can be defined partially, e.g. `2018-01` for
-`2018-01-01T00:00:00`.
-
-In ISO form, serial units are structured in text form always counted in the conventional form year-month-day etc.
-This format is based on ISO 8601 (YYYY-MM, YYYY-MM-DD, YYYY-MM-DDTHH, etc.), with some additional convention:
-* Weeks are represented as; 2018W03 (third week of 2018:, 2018-01-15 to 2018-01-21)
-* Quarters as 2018Q2 (second quarter of 2018, i.e. 2018-04 to 2018-06)
-* Semesters as 2018S2 (second semester of 2018, i.e. 2018-07 to 2018-12)
-* Trimesters as 2018t2 (second trimester of 2018, i.e. 2018-05 to 2018-08)
-* Decades as D201 (decade 201, i.e. 2010 to 2019)
-* Centuries as C21 (21st century, ie. 2001 to 2100)
-* Millenniums as M3 (3rd millinnium: 2001 to 3000)
-
 #### Limitations:
 * The iso text format does not admit `starting` or `count` parameters
 * Cyclic units (day of the week, etc.) don't admit `count` or `starting` either.