feat(schema-compiler): Add flag for using named timezones in MySQL Qu…

…ery class (#9111) * Remove moment-timezone from vertica driver * Remove moment-timezone from druid driver * Remove moment-timezone from dremio driver * Remove moment-timezone from cubestore driver * update moment-timezone in api-gateway * update moment-timezone in backend-shared * Remove moment-timezone from query-orchestrator * update moment-timezone in schema-compiler * remove unused * remove moment.HTML5_FMT.DATETIME_LOCAL_MS in favor of just literal * linter fix * add mysqlUseNamedTimezones flag * fix dremio test * Tiny edits --------- Co-authored-by: Igor Lukanin <[email protected]>
cube-js · Jan 24, 2025 · 5a540db · 5a540db
1 parent e7fd576
commit 5a540db
Show file tree

Hide file tree

Showing 19 changed files with 90 additions and 36 deletions.
diff --git a/docs/pages/product/configuration/data-sources/mysql.mdx b/docs/pages/product/configuration/data-sources/mysql.mdx
@@ -38,6 +38,7 @@ CUBEJS_DB_PASS=**********
 | `CUBEJS_DB_SSL`         | If `true`, enables SSL encryption for database connections from Cube                | `true`, `false`           |    ❌    |
 | `CUBEJS_CONCURRENCY`    | The number of concurrent connections each queue has to the database. Default is `2` | A valid number            |    ❌    |
 | `CUBEJS_DB_MAX_POOL`    | The maximum number of concurrent database connections to pool. Default is `8`       | A valid number            |    ❌    |
+| `CUBEJS_DB_MYSQL_USE_NAMED_TIMEZONES` | The flag to use time zone names or numeric offsets for time zone conversion. Default is `false` | `true`, `false` | ❌ |
 
 ## Pre-Aggregation Feature Support
 

diff --git a/docs/pages/reference/configuration/environment-variables.mdx b/docs/pages/reference/configuration/environment-variables.mdx
@@ -663,6 +663,17 @@ The cluster name to use when connecting to [Materialize](/product/configuration/
 | --------------------------------------------------------- | ---------------------- | --------------------- |
 | A valid Materialize cluster name                          | N/A                    | N/A                   |
 
+## `CUBEJS_DB_MYSQL_USE_NAMED_TIMEZONES`
+
+This flag controls how time zones conversion is done in the generated SQL for MySQL:
+- If it is set to `true`, time zone names are used. In this case, your MySQL server needs
+to be [configured][mysql-server-tz-support] properly.
+- If it is set to `false`, numeric offsets are used instead.
+
+| Possible Values | Default in Development | Default in Production |
+| --------------- | ---------------------- | --------------------- |
+| `true`, `false` | `false`                | `false`               |
+
 ## `CUBEJS_DB_SNOWFLAKE_ACCOUNT`
 
 The Snowflake account identifier to use when connecting to the database.
@@ -1612,3 +1623,4 @@ The port for a Cube deployment to listen to API connections on.
 [ref-sql-api]: /product/apis-integrations/sql-api
 [ref-sql-api-streaming]: /product/apis-integrations/sql-api#streaming
 [ref-row-limit]: /product/apis-integrations/queries#row-limit
+[mysql-server-tz-support]: https://dev.mysql.com/doc/refman/8.4/en/time-zone-support.html
diff --git a/packages/cubejs-api-gateway/package.json b/packages/cubejs-api-gateway/package.json
@@ -43,7 +43,7 @@
     "jsonwebtoken": "^9.0.2",
     "jwk-to-pem": "^2.0.4",
     "moment": "^2.24.0",
-    "moment-timezone": "^0.5.27",
+    "moment-timezone": "^0.5.46",
     "nexus": "^1.1.0",
     "node-fetch": "^2.6.1",
     "ramda": "^0.27.0",

diff --git a/packages/cubejs-backend-shared/package.json b/packages/cubejs-backend-shared/package.json
@@ -44,7 +44,7 @@
     "fs-extra": "^9.1.0",
     "http-proxy-agent": "^4.0.1",
     "moment-range": "^4.0.1",
-    "moment-timezone": "^0.5.33",
+    "moment-timezone": "^0.5.46",
     "node-fetch": "^2.6.1",
     "shelljs": "^0.8.5",
     "throttle-debounce": "^3.0.1",

diff --git a/packages/cubejs-backend-shared/src/env.ts b/packages/cubejs-backend-shared/src/env.ts
@@ -866,6 +866,44 @@ const variables: Record<string, (...args: any) => any> = {
     return undefined;
   },
 
+  /** ****************************************************************
+   * MySQL Driver                                                    *
+   ***************************************************************** */
+
+  /**
+   * Use timezone names for date/time conversions.
+   * Defaults to FALSE, meaning that numeric offsets for timezone will be used.
+   * @see https://dev.mysql.com/doc/refman/8.4/en/date-and-time-functions.html#function_convert-tz
+   * @see https://dev.mysql.com/doc/refman/8.4/en/time-zone-support.html
+   */
+  mysqlUseNamedTimezones: ({ dataSource }: { dataSource: string }) => {
+    const val = process.env[
+      keyByDataSource(
+        'CUBEJS_DB_MYSQL_USE_NAMED_TIMEZONES',
+        dataSource,
+      )
+    ];
+
+    if (val) {
+      if (val.toLocaleLowerCase() === 'true') {
+        return true;
+      } else if (val.toLowerCase() === 'false') {
+        return false;
+      } else {
+        throw new TypeError(
+          `The ${
+            keyByDataSource(
+              'CUBEJS_DB_MYSQL_USE_NAMED_TIMEZONES',
+              dataSource,
+            )
+          } must be either 'true' or 'false'.`
+        );
+      }
+    } else {
+      return false;
+    }
+  },
+
   /** ****************************************************************
    * Databricks Driver                                               *
    ***************************************************************** */

diff --git a/packages/cubejs-cubestore-driver/package.json b/packages/cubejs-cubestore-driver/package.json
@@ -33,7 +33,6 @@
     "flatbuffers": "23.3.3",
     "fs-extra": "^9.1.0",
     "generic-pool": "^3.6.0",
-    "moment-timezone": "^0.5.31",
     "node-fetch": "^2.6.1",
     "sqlstring": "^2.3.3",
     "tempy": "^1.0.1",

diff --git a/packages/cubejs-dremio-driver/driver/DremioQuery.js b/packages/cubejs-dremio-driver/driver/DremioQuery.js
@@ -1,4 +1,3 @@
-const moment = require('moment-timezone');
 const { BaseFilter, BaseQuery } = require('@cubejs-backend/schema-compiler');
 
 const GRANULARITY_TO_INTERVAL = {
@@ -36,17 +35,23 @@ class DremioQuery extends BaseQuery {
     return new DremioFilter(this, filter);
   }
 
+  /**
+   * CONVERT_TIMEZONE([sourceTimezone string], destinationTimezone string,
+   *    timestamp date, timestamp, or string in ISO 8601 format) → timestamp
+   * sourceTimezone (optional): The time zone of the timestamp. If you omit this parameter,
+   *    Dremio assumes that the source time zone is UTC.
+   * @see https://docs.dremio.com/cloud/reference/sql/sql-functions/functions/CONVERT_TIMEZONE/
+   */
   convertTz(field) {
-    const targetTZ = moment().tz(this.timezone).format('Z');
-    return `CONVERT_TIMEZONE('${targetTZ}', ${field})`;
+    return `CONVERT_TIMEZONE('${this.timezone}', ${field})`;
   }
 
   timeStampCast(value) {
     return `TO_TIMESTAMP(${value}, 'YYYY-MM-DD"T"HH24:MI:SS.FFF')`;
   }
 
   timestampFormat() {
-    return moment.HTML5_FMT.DATETIME_LOCAL_MS;
+    return 'YYYY-MM-DDTHH:mm:ss.SSS';
   }
 
   dateTimeCast(value) {

diff --git a/packages/cubejs-dremio-driver/package.json b/packages/cubejs-dremio-driver/package.json
@@ -26,7 +26,6 @@
     "@cubejs-backend/schema-compiler": "1.1.16",
     "@cubejs-backend/shared": "1.1.12",
     "axios": "^0.21.1",
-    "moment-timezone": "^0.5.31",
     "sqlstring": "^2.3.1"
   },
   "devDependencies": {

diff --git a/packages/cubejs-dremio-driver/test/DremioQuery.test.ts b/packages/cubejs-dremio-driver/test/DremioQuery.test.ts
@@ -63,7 +63,7 @@ cube(\`sales\`, {
     const queryAndParams = query.buildSqlAndParams();
 
     expect(queryAndParams[0]).toContain(
-      'DATE_TRUNC(\'day\', CONVERT_TIMEZONE(\'-08:00\', "sales".sales_datetime))'
+      'DATE_TRUNC(\'day\', CONVERT_TIMEZONE(\'America/Los_Angeles\', "sales".sales_datetime))'
     );
   }));
 

diff --git a/packages/cubejs-druid-driver/package.json b/packages/cubejs-druid-driver/package.json
@@ -31,8 +31,7 @@
     "@cubejs-backend/base-driver": "1.1.16",
     "@cubejs-backend/schema-compiler": "1.1.16",
     "@cubejs-backend/shared": "1.1.12",
-    "axios": "^0.21.1",
-    "moment-timezone": "^0.5.31"
+    "axios": "^0.21.1"
   },
   "devDependencies": {
     "@cubejs-backend/linter": "^1.0.0",

diff --git a/packages/cubejs-druid-driver/src/DruidQuery.ts b/packages/cubejs-druid-driver/src/DruidQuery.ts
@@ -1,4 +1,3 @@
-import moment from 'moment-timezone';
 import { BaseFilter, BaseQuery } from '@cubejs-backend/schema-compiler';
 
 const GRANULARITY_TO_INTERVAL: Record<string, (date: string) => string> = {

diff --git a/packages/cubejs-query-orchestrator/package.json b/packages/cubejs-query-orchestrator/package.json
@@ -36,7 +36,6 @@
     "es5-ext": "0.10.53",
     "generic-pool": "^3.7.1",
     "lru-cache": "^6.0.0",
-    "moment-timezone": "^0.5.33",
     "ramda": "^0.27.2"
   },
   "devDependencies": {

diff --git a/packages/cubejs-schema-compiler/package.json b/packages/cubejs-schema-compiler/package.json
@@ -50,7 +50,7 @@
     "joi": "^17.8.3",
     "js-yaml": "^4.1.0",
     "lru-cache": "^5.1.1",
-    "moment-timezone": "^0.5.33",
+    "moment-timezone": "^0.5.46",
     "node-dijkstra": "^2.5.0",
     "ramda": "^0.27.2",
     "syntax-error": "^1.3.0",

diff --git a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js
@@ -2818,10 +2818,6 @@ export class BaseQuery {
     return this.join && this.join.multiplicationFactor[cubeName];
   }
 
-  inIntegrationTimeZone(date) {
-    return moment.tz(date, this.timezone);
-  }
-
   inDbTimeZone(date) {
     return inDbTimeZone(this.timezone, this.timestampFormat(), date);
   }

diff --git a/packages/cubejs-schema-compiler/src/adapter/CubeStoreQuery.ts b/packages/cubejs-schema-compiler/src/adapter/CubeStoreQuery.ts
@@ -47,7 +47,7 @@ export class CubeStoreQuery extends BaseQuery {
   }
 
   public timestampFormat() {
-    return moment.HTML5_FMT.DATETIME_LOCAL_MS;
+    return 'YYYY-MM-DDTHH:mm:ss.SSS';
   }
 
   public dateTimeCast(value) {

diff --git a/packages/cubejs-schema-compiler/src/adapter/MysqlQuery.ts b/packages/cubejs-schema-compiler/src/adapter/MysqlQuery.ts
@@ -1,7 +1,5 @@
 import moment from 'moment-timezone';
-
-import { parseSqlInterval } from '@cubejs-backend/shared';
-
+import { getEnv, parseSqlInterval } from '@cubejs-backend/shared';
 import { BaseQuery } from './BaseQuery';
 import { BaseFilter } from './BaseFilter';
 import { UserError } from '../compiler/UserError';
@@ -26,39 +24,50 @@ class MysqlFilter extends BaseFilter {
 }
 
 export class MysqlQuery extends BaseQuery {
+  private readonly useNamedTimezones: boolean;
+
+  public constructor(compilers: any, options: any) {
+    super(compilers, options);
+
+    this.useNamedTimezones = getEnv('mysqlUseNamedTimezones', { dataSource: this.dataSource });
+  }
+
   public newFilter(filter) {
     return new MysqlFilter(this, filter);
   }
 
-  public castToString(sql) {
+  public castToString(sql: string) {
     return `CAST(${sql} as CHAR)`;
   }
 
-  public convertTz(field) {
+  public convertTz(field: string) {
+    if (this.useNamedTimezones) {
+      return `CONVERT_TZ(${field}, @@session.time_zone, '${this.timezone}')`;
+    }
     return `CONVERT_TZ(${field}, @@session.time_zone, '${moment().tz(this.timezone).format('Z')}')`;
   }
 
-  public timeStampCast(value) {
+  public timeStampCast(value: string) {
     return `TIMESTAMP(convert_tz(${value}, '+00:00', @@session.time_zone))`;
   }
 
   public timestampFormat() {
-    return moment.HTML5_FMT.DATETIME_LOCAL_MS;
+    return 'YYYY-MM-DDTHH:mm:ss.SSS';
   }
 
-  public dateTimeCast(value) {
+  public dateTimeCast(value: string) {
     return `TIMESTAMP(${value})`;
   }
 
-  public subtractInterval(date, interval) {
+  public subtractInterval(date: string, interval: string) {
     return `DATE_SUB(${date}, INTERVAL ${this.formatInterval(interval)})`;
   }
 
-  public addInterval(date, interval) {
+  public addInterval(date: string, interval: string) {
     return `DATE_ADD(${date}, INTERVAL ${this.formatInterval(interval)})`;
   }
 
-  public timeGroupedColumn(granularity, dimension) {
+  public timeGroupedColumn(granularity: string, dimension) {
     return `CAST(${GRANULARITY_TO_INTERVAL[granularity](dimension)} AS DATETIME)`;
   }
 

diff --git a/packages/cubejs-vertica-driver/package.json b/packages/cubejs-vertica-driver/package.json
@@ -22,7 +22,6 @@
     "@cubejs-backend/base-driver": "1.1.16",
     "@cubejs-backend/query-orchestrator": "1.1.16",
     "@cubejs-backend/schema-compiler": "1.1.16",
-    "moment-timezone": "^0.5.45",
     "vertica-nodejs": "^1.0.3"
   },
   "license": "Apache-2.0",

diff --git a/packages/cubejs-vertica-driver/src/VerticaQuery.js b/packages/cubejs-vertica-driver/src/VerticaQuery.js
@@ -1,4 +1,3 @@
-const moment = require('moment-timezone');
 const { BaseFilter, BaseQuery } = require('@cubejs-backend/schema-compiler');
 
 const GRANULARITY_TO_INTERVAL = {
@@ -38,7 +37,7 @@ class VerticaQuery extends BaseQuery {
   }
 
   timestampFormat() {
-    return moment.HTML5_FMT.DATETIME_LOCAL_MS;
+    return 'YYYY-MM-DDTHH:mm:ss.SSS';
   }
 
   dateTimeCast(value) {

diff --git a/yarn.lock b/yarn.lock
@@ -21779,16 +21779,16 @@ moment-range@*, moment-range@^4.0.1:
   dependencies:
     es6-symbol "^3.1.0"
 
-moment-timezone@^0.5.15, moment-timezone@^0.5.27, moment-timezone@^0.5.31, moment-timezone@^0.5.33:
+moment-timezone@^0.5.15, moment-timezone@^0.5.33:
   version "0.5.45"
   resolved "https://registry.yarnpkg.com/moment-timezone/-/moment-timezone-0.5.45.tgz#cb685acd56bac10e69d93c536366eb65aa6bcf5c"
   integrity sha512-HIWmqA86KcmCAhnMAN0wuDOARV/525R2+lOLotuGFzn4HO+FH+/645z2wx0Dt3iDv6/p61SIvKnDstISainhLQ==
   dependencies:
     moment "^2.29.4"
 
-moment-timezone@^0.5.45:
+moment-timezone@^0.5.46:
   version "0.5.46"
-  resolved "https://registry.npmjs.org/moment-timezone/-/moment-timezone-0.5.46.tgz#a21aa6392b3c6b3ed916cd5e95858a28d893704a"
+  resolved "https://registry.yarnpkg.com/moment-timezone/-/moment-timezone-0.5.46.tgz#a21aa6392b3c6b3ed916cd5e95858a28d893704a"
   integrity sha512-ZXm9b36esbe7OmdABqIWJuBBiLLwAjrN7CE+7sYdCCx82Nabt1wHDj8TVseS59QIlfFPbOoiBPm6ca9BioG4hw==
   dependencies:
     moment "^2.29.4"