fix: Mismatched input '10000'. Expecting: '?', 'ALL', <integer> for p…

…ost-aggregate members in Athena (#8262) * fix: Mismatched input '10000'. Expecting: '?', 'ALL', <integer> for post-aggregate members in Athena * Missing connection definition * Remove credentials duplication * Fix precision * Fix presto offset test
cube-js · May 15, 2024 · 59834e7 · 59834e7
1 parent e559114
commit 59834e7
Show file tree

Hide file tree

Showing 14 changed files with 336 additions and 15 deletions.
diff --git a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js
@@ -1981,15 +1981,27 @@ export class BaseQuery {
   }
 
   groupByDimensionLimit() {
-    let limitClause = '';
+    let limit = null;
     if (this.rowLimit !== null) {
       if (this.rowLimit === MAX_SOURCE_ROW_LIMIT) {
-        limitClause = ` LIMIT ${this.paramAllocator.allocateParam(MAX_SOURCE_ROW_LIMIT)}`;
+        limit = this.paramAllocator.allocateParam(MAX_SOURCE_ROW_LIMIT);
       } else if (typeof this.rowLimit === 'number') {
-        limitClause = ` LIMIT ${this.rowLimit}`;
+        limit = this.rowLimit;
       }
     }
-    const offsetClause = this.offset ? ` OFFSET ${parseInt(this.offset, 10)}` : '';
+    const offset = this.offset ? parseInt(this.offset, 10) : null;
+    return this.limitOffsetClause(limit, offset);
+  }
+
+  /**
+   * @protected
+   * @param limit
+   * @param offset
+   * @returns {string}
+   */
+  limitOffsetClause(limit, offset) {
+    const limitClause = limit != null ? ` LIMIT ${limit}` : '';
+    const offsetClause = offset != null ? ` OFFSET ${offset}` : '';
     return `${limitClause}${offsetClause}`;
   }
 

diff --git a/packages/cubejs-schema-compiler/src/adapter/MssqlQuery.ts b/packages/cubejs-schema-compiler/src/adapter/MssqlQuery.ts
@@ -78,6 +78,7 @@ export class MssqlQuery extends BaseQuery {
     return new MssqlParamAllocator(expressionParams);
   }
 
+  // TODO replace with limitOffsetClause override
   public groupByDimensionLimit() {
     if (this.rowLimit) {
       return this.offset ? ` OFFSET ${parseInt(this.offset, 10)} ROWS FETCH NEXT ${parseInt(this.rowLimit, 10)} ROWS ONLY` : '';

diff --git a/packages/cubejs-schema-compiler/src/adapter/OracleQuery.ts b/packages/cubejs-schema-compiler/src/adapter/OracleQuery.ts
@@ -31,6 +31,7 @@ class OracleFilter extends BaseFilter {
 export class OracleQuery extends BaseQuery {
   /**
    * "LIMIT" on Oracle it's illegal
+   * TODO replace with limitOffsetClause override
    */
   public groupByDimensionLimit() {
     const limitClause = this.rowLimit === null ? '' : ` FETCH NEXT ${this.rowLimit && parseInt(this.rowLimit, 10) || 10000} ROWS ONLY`;

diff --git a/packages/cubejs-schema-compiler/src/adapter/PrestodbQuery.ts b/packages/cubejs-schema-compiler/src/adapter/PrestodbQuery.ts
@@ -102,10 +102,9 @@ export class PrestodbQuery extends BaseQuery {
     return `approx_distinct(${sql})`;
   }
 
-  public groupByDimensionLimit() {
-    const limitClause = this.rowLimit === null ? '' : ` LIMIT ${this.rowLimit && parseInt(this.rowLimit, 10) || 10000}`;
-    const offsetClause = this.offset ? ` OFFSET ${parseInt(this.offset, 10)}` : '';
-
+  protected limitOffsetClause(limit, offset) {
+    const limitClause = limit != null ? ` LIMIT ${limit}` : '';
+    const offsetClause = offset != null ? ` OFFSET ${offset}` : '';
     return `${offsetClause}${limitClause}`;
   }
 

diff --git a/packages/cubejs-schema-compiler/test/integration/postgres/sql-generation.test.ts b/packages/cubejs-schema-compiler/test/integration/postgres/sql-generation.test.ts
@@ -285,7 +285,7 @@ describe('SQL Generation', () => {
     view('visitors_post_aggregate', {
       cubes: [{
         join_path: 'visitors',
-        includes: ['adjusted_rank_sum', 'source', 'updated_at', 'visitor_revenue']
+        includes: '*'
       }]
     })
 
@@ -957,6 +957,7 @@ describe('SQL Generation', () => {
       }],
       timezone: 'America/Los_Angeles',
       offset: 5,
+      rowLimit: 5,
     });
 
     console.log(query.buildSqlAndParams());
@@ -2596,6 +2597,32 @@ describe('SQL Generation', () => {
     }]
   ));
 
+  it('post aggregate percentage of total with limit', async () => runQueryTest(
+    {
+      measures: ['visitors_post_aggregate.percentage_of_total'],
+      dimensions: ['visitors_post_aggregate.source'],
+      order: [{
+        id: 'visitors_post_aggregate.source'
+      }],
+      rowLimit: 1,
+      limit: 1
+    },
+    [{
+      visitors_post_aggregate__percentage_of_total: 15,
+      visitors_post_aggregate__source: 'google'
+    }]
+  ));
+
+  it('post aggregate percentage of total with limit totals', async () => runQueryTest(
+    {
+      measures: ['visitors_post_aggregate.percentage_of_total'],
+      rowLimit: 1
+    },
+    [{
+      visitors_post_aggregate__percentage_of_total: 100
+    }]
+  ));
+
   it('post aggregate percentage of total filtered', async () => runQueryTest(
     {
       measures: ['visitors.revenue', 'visitors.percentage_of_total'],

diff --git a/packages/cubejs-testing-drivers/fixtures/_schemas.json b/packages/cubejs-testing-drivers/fixtures/_schemas.json
@@ -290,6 +290,19 @@
             "type": "prior"
           }]
         },
+        {
+          "name": "totalProfitForStatus",
+          "type": "sum",
+          "sql": "{totalProfit}",
+          "post_aggregate": true,
+          "reduce_by": ["category"]
+        },
+        {
+          "name": "percentageOfTotalForStatus",
+          "type": "number",
+          "sql": "ROUND(100 * {totalProfit} / NULLIF({totalProfitForStatus}, 0))",
+          "post_aggregate": true
+        },
         {
           "name": "hiddenSum",
           "sql": "profit",

diff --git a/packages/cubejs-testing-drivers/fixtures/athena.json b/packages/cubejs-testing-drivers/fixtures/athena.json
@@ -11,9 +11,13 @@
       "CUBEJS_AWS_SECRET": "${DRIVERS_TESTS_ATHENA_CUBEJS_AWS_SECRET}",
       "CUBEJS_AWS_REGION": "us-east-1",
       "CUBEJS_AWS_ATHENA_WORKGROUP": "primary",
-      "CUBEJS_AWS_S3_OUTPUT_LOCATION": "s3://athena-drivers-tests-preaggs/output"
+      "CUBEJS_AWS_S3_OUTPUT_LOCATION": "s3://athena-drivers-tests-preaggs/output",
+      "CUBEJS_PG_SQL_PORT": "5656",
+      "CUBEJS_SQL_USER": "admin",
+      "CUBEJS_SQL_PASSWORD": "admin_password",
+      "CUBESQL_SQL_PUSH_DOWN": "true"
     },
-    "ports" : ["4000"]
+    "ports" : ["4000", "5656"]
   },
   "cast": {
     "SELECT_PREFIX": "",

diff --git a/packages/cubejs-testing-drivers/src/tests/testQueries.ts b/packages/cubejs-testing-drivers/src/tests/testQueries.ts
@@ -32,7 +32,7 @@ export function testQueries(type: string, { includeIncrementalSchemaSuite, exten
 
     let connectionId = 0;
 
-    async function createPostgresClient(user: string, password: string, pgPort: number | undefined) {
+    async function createPostgresClient(user: string = 'admin', password: string = 'admin_password', pgPort: number | undefined = env.cube.pgPort) {
       if (!pgPort) {
         throw new Error('port must be defined');
       }
@@ -1513,7 +1513,7 @@ export function testQueries(type: string, { includeIncrementalSchemaSuite, exten
     }
 
     executePg('SQL API: powerbi min max push down', async () => {
-      const connection = await createPostgresClient('admin', 'admin_password', env.cube.pgPort);
+      const connection = await createPostgresClient();
       const res = await connection.query(`
       select
   max("rows"."orderDate") as "a0",
@@ -1530,7 +1530,7 @@ from
     });
 
     executePg('SQL API: powerbi min max ungrouped flag', async () => {
-      const connection = await createPostgresClient('admin', 'admin_password', env.cube.pgPort);
+      const connection = await createPostgresClient();
       const res = await connection.query(`
       select 
   count(distinct("rows"."totalSales")) + max( 
@@ -1553,7 +1553,7 @@ from
     });
 
     executePg('SQL API: ungrouped pre-agg', async () => {
-      const connection = await createPostgresClient('admin', 'admin_password', env.cube.pgPort);
+      const connection = await createPostgresClient();
       const res = await connection.query(`
     select
       "productName",
@@ -1564,5 +1564,16 @@ from
   `);
       expect(res.rows).toMatchSnapshot('ungrouped_pre_agg');
     });
+
+    executePg('SQL API: post-aggregate percentage of total', async () => {
+      const connection = await createPostgresClient();
+      const res = await connection.query(`
+    select
+      sum("BigECommerce"."percentageOfTotalForStatus")
+    from 
+      "public"."BigECommerce" "BigECommerce"
+  `);
+      expect(res.rows).toMatchSnapshot('post_aggregate_percentage_of_total');
+    });
   });
 }