cube-js / cube

📊 Cube — Universal semantic layer platform for AI, BI, spreadsheets, and embedded analytics
https://cube.dev
Other
17.97k stars 1.78k forks source link

Default Refresh Key SQL Seems to Generate Incorrectly for MSSQL #3629

Open bgiles28 opened 3 years ago

bgiles28 commented 3 years ago

Describe the bug When I don't specify a sql parameter in my preaggregation refreshKey, the the default refresh key that seems to be generated is invalid for MSSQL.

To Reproduce Steps to reproduce the behavior:

  1. In the .env file, specify CUBEJS_DB_TYPE=mssql
  2. Setup a preaggregation with the following configuration:
    myAggregation: {
      refreshKey: {
        every: `1 minute`,
        updateWindow: `12 week`
      },
      type: `rollup`,
      external: true,
      scheduledRefresh: true,
      measureReferences: [...],
      dimensionReferences: [...],
      granularity: `month`,
      partitionGranularity: `quarter`,
      buildRangeStart: {
        sql: `SELECT CAST('2020-10-01' AS DATETIME2)`
      },
      buildRangeEnd: {
        sql: `Select GETDATE()`
      }
    }
  3. After spinning up a cube instance, the log output reads as follows:
    Executing SQL: scheduler-85bb3bf2-32a9-4e30-bc0f-50a68ba092a3 
    --
    SELECT FLOOR((UNIX_TIMESTAMP()) / 60) as refresh_key
    --
    Executing SQL: scheduler-85bb3bf2-32a9-4e30-bc0f-50a68ba092a3 
    --
    SELECT FLOOR((UNIX_TIMESTAMP()) / 60) as refresh_key
    --
    Performing query: scheduler-85bb3bf2-32a9-4e30-bc0f-50a68ba092a3 
    Executing SQL: scheduler-85bb3bf2-32a9-4e30-bc0f-50a68ba092a3 
    --
    SELECT FLOOR((UNIX_TIMESTAMP()) / 60) as refresh_key

And, when I instead specify my own sql refreshKey such as SELECT DATEDIFF(SECOND, '19700101', sysutcdatetime()) / 60, the logs show this valid MSSQL being executed. I would like to be able to leverage the "incremental" and "updateWindow" capabilities, but as explained in this question: https://github.com/cube-js/cube.js/issues/3585, it's not currently possible to both specify custom sql and leverage these parameters. So in order to be able to use parameters like updateWindow and incremental, the default refreshKey that gets generated needs to be a valid MSSQL statement.

Expected behavior A valid MSSQL query is generated as the default refreshKey. Looking at the MssqlQuery adapater, it seems like the components are all there for the query to be generated correctly, so I'm unsure why the logs are indicating otherwise. The adapter correctly implements valid MSSQL for the unix timestamp functions: https://github.com/cube-js/cube.js/blob/master/packages/cubejs-schema-compiler/src/adapter/MssqlQuery.js#L111-L118

Version: "@cubejs-backend/cubestore-driver": "0.28.53", "@cubejs-backend/mssql-driver": "0.28.52", "@cubejs-backend/server": "0.28.53"

github-actions[bot] commented 3 years ago

If you are interested in working on this issue, please leave a comment below and we will be happy to assign the issue to you. If this is the first time you are contributing a Pull Request to Cube.js, please check our contribution guidelines. You can also post any questions while contributing in the #contributors channel in the Cube.js Slack.