From ea3ba21b13b8b2c3409588c15eb822e6f76ba040 Mon Sep 17 00:00:00 2001 From: Tom Tankilevitch Date: Sun, 19 Oct 2025 17:25:33 +0300 Subject: [PATCH 1/4] fix(schema-compiler): remove hardcoded AS keyword in subquery aliases for Oracle compatibility Oracle database does not support the AS keyword for table/subquery aliasing, while other databases like PostgreSQL and MySQL do. The existing BaseQuery implementation hardcoded 'as' in subquery alias generation, causing Oracle queries to fail. This change: - Replaces hardcoded 'as' with asSyntaxJoin property in BaseQuery - Oracle returns empty string for asSyntaxJoin (no AS keyword) - PostgreSQL/MySQL return 'AS' (maintains existing behavior) - Adds comprehensive Oracle query test suite validating AS syntax removal - Adds PostgreSQL regression test ensuring AS keyword is still present This fixes queries with rolling windows and multiple subqueries on Oracle, which previously generated invalid SQL like: SELECT ... FROM (...) as q_0 INNER JOIN (...) as q_1 ON ... Now Oracle correctly generates: SELECT ... FROM (...) q_0 INNER JOIN (...) q_1 ON ... --- .../src/adapter/BaseQuery.js | 6 +- .../test/unit/oracle-query.test.ts | 308 ++++++++++++++++++ .../test/unit/postgres-query.test.ts | 28 ++ 3 files changed, 339 insertions(+), 3 deletions(-) create mode 100644 packages/cubejs-schema-compiler/test/unit/oracle-query.test.ts diff --git a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js index 24b8d4ffa380b..3c99404ff9c3e 100644 --- a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js +++ b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js @@ -1380,8 +1380,8 @@ export class BaseQuery { const join = R.drop(1, toJoin) .map( (q, i) => (this.dimensionAliasNames().length ? - `INNER JOIN ${this.wrapInParenthesis((q))} as q_${i + 1} ON ${this.dimensionsJoinCondition(`q_${i}`, `q_${i + 1}`)}` : - `, ${this.wrapInParenthesis(q)} as q_${i + 1}`), + `INNER JOIN ${this.wrapInParenthesis((q))} ${this.asSyntaxJoin} q_${i + 1} ON ${this.dimensionsJoinCondition(`q_${i}`, `q_${i + 1}`)}` : + `, ${this.wrapInParenthesis(q)} ${this.asSyntaxJoin} q_${i + 1}`), ).join('\n'); const columnsToSelect = this.evaluateSymbolSqlWithContext( @@ -1410,7 +1410,7 @@ export class BaseQuery { return `${toJoin[0].replace(/^SELECT/, `SELECT ${this.topLimit()}`)} ${this.orderBy()}${this.groupByDimensionLimit()}`; } - return `SELECT ${this.topLimit()}${columnsToSelect} FROM ${this.wrapInParenthesis(toJoin[0])} as q_0 ${join}${havingFilters}${this.orderBy()}${this.groupByDimensionLimit()}`; + return `SELECT ${this.topLimit()}${columnsToSelect} FROM ${this.wrapInParenthesis(toJoin[0])} ${this.asSyntaxJoin} q_0 ${join}${havingFilters}${this.orderBy()}${this.groupByDimensionLimit()}`; } wrapInParenthesis(select) { diff --git a/packages/cubejs-schema-compiler/test/unit/oracle-query.test.ts b/packages/cubejs-schema-compiler/test/unit/oracle-query.test.ts new file mode 100644 index 0000000000000..73aac1e890ccb --- /dev/null +++ b/packages/cubejs-schema-compiler/test/unit/oracle-query.test.ts @@ -0,0 +1,308 @@ +/* eslint-disable no-restricted-syntax */ +import { OracleQuery } from '../../src/adapter/OracleQuery'; +import { prepareJsCompiler } from './PrepareCompiler'; + +describe('OracleQuery', () => { + const { compiler, joinGraph, cubeEvaluator } = prepareJsCompiler(` + cube(\`visitors\`, { + sql: \` + select * from visitors + \`, + + measures: { + count: { + type: 'count' + }, + + unboundedCount: { + type: 'count', + rollingWindow: { + trailing: 'unbounded' + } + }, + + thisPeriod: { + sql: 'amount', + type: 'sum', + rollingWindow: { + trailing: '1 year', + offset: 'end' + } + }, + + priorPeriod: { + sql: 'amount', + type: 'sum', + rollingWindow: { + trailing: '1 year', + offset: 'start' + } + } + }, + + dimensions: { + id: { + sql: 'id', + type: 'number', + primaryKey: true + }, + + createdAt: { + type: 'time', + sql: 'created_at' + }, + + source: { + type: 'string', + sql: 'source' + } + } + }) + + cube(\`Deals\`, { + sql: \`select * from deals\`, + + measures: { + amount: { + sql: \`amount\`, + type: \`sum\` + } + }, + + dimensions: { + salesManagerId: { + sql: \`sales_manager_id\`, + type: 'string', + primaryKey: true + } + } + }) + + cube(\`SalesManagers\`, { + sql: \`select * from sales_managers\`, + + joins: { + Deals: { + relationship: \`hasMany\`, + sql: \`\${SalesManagers}.id = \${Deals}.sales_manager_id\` + } + }, + + measures: { + averageDealAmount: { + sql: \`\${dealsAmount}\`, + type: \`avg\` + } + }, + + dimensions: { + id: { + sql: \`id\`, + type: \`string\`, + primaryKey: true + }, + + dealsAmount: { + sql: \`\${Deals.amount}\`, + type: \`number\`, + subQuery: true + } + } + }); + `, { adapter: 'oracle' }); + + it('basic query without subqueries', async () => { + await compiler.compile(); + + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: [ + 'visitors.count' + ], + timeDimensions: [], + timezone: 'UTC' + }); + + const queryAndParams = query.buildSqlAndParams(); + const sql = queryAndParams[0]; + + // Basic query should work + expect(sql).toContain('SELECT'); + expect(sql).toMatch(/FROM\s+visitors/i); + // Should not have subquery aliases in simple query + expect(sql).not.toMatch(/\bq_\d+\b/); + // Should use Oracle FETCH NEXT + expect(sql).toContain('FETCH NEXT'); + }); + + it('does not use AS keyword in subquery aliases with single rolling window', async () => { + await compiler.compile(); + + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: [ + 'visitors.count', + 'visitors.unboundedCount' + ], + timeDimensions: [{ + dimension: 'visitors.createdAt', + granularity: 'day', + dateRange: ['2020-01-01', '2020-01-31'] + }], + timezone: 'UTC' + }); + + const queryAndParams = query.buildSqlAndParams(); + const sql = queryAndParams[0]; + + // Oracle should NOT have AS keyword before subquery aliases + expect(sql).not.toMatch(/\bAS\s+q_\d+/i); + expect(sql).not.toMatch(/\bas\s+q_\d+/); + + // Should have q_0 alias (with space around it, indicating no AS) + expect(sql).toMatch(/\)\s+q_0\s+/); + }); + + it('does not use AS keyword with multiple rolling window measures (YoY scenario)', async () => { + await compiler.compile(); + + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: [ + 'visitors.thisPeriod', + 'visitors.priorPeriod' + ], + timeDimensions: [{ + dimension: 'visitors.createdAt', + granularity: 'year', + dateRange: ['2020-01-01', '2022-12-31'] + }], + timezone: 'UTC' + }); + + const queryAndParams = query.buildSqlAndParams(); + const sql = queryAndParams[0]; + + // Should have multiple subquery aliases (q_0, q_1, q_2, etc.) + expect(sql).toMatch(/\bq_0\b/); + expect(sql).toMatch(/\bq_1\b/); + + // Oracle should NOT have AS keyword anywhere before q_ aliases + expect(sql).not.toMatch(/\bAS\s+q_\d+/i); + expect(sql).not.toMatch(/\bas\s+q_\d+/); + + // Verify pattern is ) q_X not ) AS q_X + expect(sql).toMatch(/\)\s+q_\d+/); + }); + + it('does not use AS keyword in INNER JOIN subqueries', async () => { + await compiler.compile(); + + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + dimensions: [ + 'SalesManagers.id', + 'SalesManagers.dealsAmount' + ] + }); + + const queryAndParams = query.buildSqlAndParams(); + const sql = queryAndParams[0]; + + // Should have INNER JOIN for subquery dimension + if (sql.includes('INNER JOIN')) { + // Oracle should NOT have AS keyword in INNER JOIN + expect(sql).not.toMatch(/INNER\s+JOIN\s+\([^)]+\)\s+AS\s+/i); + expect(sql).not.toMatch(/INNER\s+JOIN\s+\([^)]+\)\s+as\s+/); + } + }); + + it('uses FETCH NEXT syntax instead of LIMIT', async () => { + await compiler.compile(); + + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: [ + 'visitors.count' + ], + timezone: 'UTC', + limit: 100 + }); + + const queryAndParams = query.buildSqlAndParams(); + const sql = queryAndParams[0]; + + // Oracle should use FETCH NEXT instead of LIMIT + expect(sql).toContain('FETCH NEXT'); + expect(sql).toContain('ROWS ONLY'); + expect(sql).not.toContain('LIMIT'); + }); + + it('uses FETCH NEXT syntax with subqueries and rolling windows', async () => { + await compiler.compile(); + + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: [ + 'visitors.thisPeriod', + 'visitors.priorPeriod' + ], + timeDimensions: [{ + dimension: 'visitors.createdAt', + granularity: 'month', + dateRange: ['2020-01-01', '2020-12-31'] + }], + timezone: 'UTC', + limit: 50 + }); + + const queryAndParams = query.buildSqlAndParams(); + const sql = queryAndParams[0]; + + // Should have subqueries without AS + expect(sql).not.toMatch(/\bAS\s+q_\d+/i); + + // Should use Oracle-specific FETCH NEXT + expect(sql).toContain('FETCH NEXT'); + expect(sql).toContain('ROWS ONLY'); + expect(sql).not.toContain('LIMIT'); + }); + + it('does not use AS keyword with comma-separated subqueries', async () => { + await compiler.compile(); + + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: [ + 'visitors.thisPeriod', + 'visitors.priorPeriod' + ], + timezone: 'UTC' + }); + + const queryAndParams = query.buildSqlAndParams(); + const sql = queryAndParams[0]; + + // Should have multiple subquery aliases + expect(sql).toMatch(/\)\s+q_0\s+,/); + expect(sql).toMatch(/\)\s+q_1\s+/); + + // Should NOT have AS before q_ aliases + expect(sql).not.toMatch(/\bAS\s+q_\d+/i); + expect(sql).not.toMatch(/\bas\s+q_\d+/); + }); + + it('group by dimensions not indexes', async () => { + await compiler.compile(); + + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: [ + 'visitors.count' + ], + dimensions: [ + 'visitors.source' + ], + timezone: 'UTC' + }); + + const queryAndParams = query.buildSqlAndParams(); + const sql = queryAndParams[0]; + + // Oracle should group by actual dimension SQL, not by index + expect(sql).toMatch(/GROUP BY.*"visitors"\.source/i); + expect(sql).not.toMatch(/GROUP BY\s+\d+/); + }); +}); diff --git a/packages/cubejs-schema-compiler/test/unit/postgres-query.test.ts b/packages/cubejs-schema-compiler/test/unit/postgres-query.test.ts index b72c0b4d97311..1bba2d1ffdd2a 100644 --- a/packages/cubejs-schema-compiler/test/unit/postgres-query.test.ts +++ b/packages/cubejs-schema-compiler/test/unit/postgres-query.test.ts @@ -328,4 +328,32 @@ describe('PostgresQuery', () => { expect(queryAndParams[0]).toContain('ORDER BY 3 ASC'); }); }); + + it('uses AS keyword in subquery aliases (regression test)', async () => { + await compiler.compile(); + + const query = new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: [ + 'visitors.count', + 'visitors.unboundedCount' + ], + timeDimensions: [{ + dimension: 'visitors.createdAt', + granularity: 'day', + dateRange: ['2020-01-01', '2020-01-31'] + }], + timezone: 'UTC' + }); + + const queryAndParams = query.buildSqlAndParams(); + const sql = queryAndParams[0]; + + // PostgreSQL should use AS keyword for subquery aliases + expect(sql).toMatch(/\s+AS\s+q_0\s+/); + + // Should NOT have pattern ) q_0 (without AS) + // This regex checks for closing paren followed by space(s), q_0, space, but NOT preceded by AS + const hasAsKeyword = /\s+AS\s+q_0\s+/.test(sql); + expect(hasAsKeyword).toBe(true); + }); }); From 076bc9f57baaabf7f2df1691713c0f9c801dffc2 Mon Sep 17 00:00:00 2001 From: Tom Tankilevitch Date: Sun, 19 Oct 2025 18:51:03 +0300 Subject: [PATCH 2/4] fix(oracle): add support for time filters and rolling windows - Fix AS keyword in subquery aliases (Oracle doesn't support it) - Handle time dimensions without granularity to prevent TypeError - Implement Oracle-specific interval arithmetic using ADD_MONTHS and NUMTODSINTERVAL - Add comprehensive test suite for Oracle query generation These changes enable Oracle users to execute queries with: - Time dimension filters without granularity specification - Rolling windows and time-based calculations - Multiple subqueries with proper aliasing All changes maintain backward compatibility with other database adapters. --- .../src/adapter/BaseQuery.js | 5 ++ .../src/adapter/OracleQuery.ts | 85 ++++++++++++++++++- .../test/unit/oracle-query.test.ts | 46 ++++++++++ 3 files changed, 135 insertions(+), 1 deletion(-) diff --git a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js index 3c99404ff9c3e..e42e1648d4bee 100644 --- a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js +++ b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js @@ -3857,6 +3857,11 @@ export class BaseQuery { * @return {string} */ dimensionTimeGroupedColumn(dimension, granularity) { + // Handle case when granularity is not specified (e.g., time dimension used only for filtering) + if (!granularity) { + return this.timeGroupedColumn(null, dimension); + } + let dtDate; // Interval is aligned with natural calendar, so we can use DATE_TRUNC diff --git a/packages/cubejs-schema-compiler/src/adapter/OracleQuery.ts b/packages/cubejs-schema-compiler/src/adapter/OracleQuery.ts index d5e4871b88497..592ce083f1d56 100644 --- a/packages/cubejs-schema-compiler/src/adapter/OracleQuery.ts +++ b/packages/cubejs-schema-compiler/src/adapter/OracleQuery.ts @@ -1,7 +1,8 @@ +import { parseSqlInterval } from '@cubejs-backend/shared'; import { BaseQuery } from './BaseQuery'; import { BaseFilter } from './BaseFilter'; import { UserError } from '../compiler/UserError'; -import { BaseDimension } from './BaseDimension'; +import type { BaseDimension } from './BaseDimension'; const GRANULARITY_VALUE = { day: 'DD', @@ -89,6 +90,88 @@ export class OracleQuery extends BaseQuery { return `TRUNC(${dimension}, '${GRANULARITY_VALUE[granularity]}')`; } + /** + * Oracle uses ADD_MONTHS for year/month/quarter intervals + * and NUMTODSINTERVAL for day/hour/minute/second intervals + */ + public addInterval(date: string, interval: string): string { + const intervalParsed = parseSqlInterval(interval); + let res = date; + + // Handle year/month/quarter using ADD_MONTHS + let totalMonths = 0; + if (intervalParsed.year) { + totalMonths += intervalParsed.year * 12; + } + if (intervalParsed.quarter) { + totalMonths += intervalParsed.quarter * 3; + } + if (intervalParsed.month) { + totalMonths += intervalParsed.month; + } + + if (totalMonths !== 0) { + res = `ADD_MONTHS(${res}, ${totalMonths})`; + } + + // Handle day/hour/minute/second using NUMTODSINTERVAL + if (intervalParsed.day) { + res = `${res} + NUMTODSINTERVAL(${intervalParsed.day}, 'DAY')`; + } + if (intervalParsed.hour) { + res = `${res} + NUMTODSINTERVAL(${intervalParsed.hour}, 'HOUR')`; + } + if (intervalParsed.minute) { + res = `${res} + NUMTODSINTERVAL(${intervalParsed.minute}, 'MINUTE')`; + } + if (intervalParsed.second) { + res = `${res} + NUMTODSINTERVAL(${intervalParsed.second}, 'SECOND')`; + } + + return res; + } + + /** + * Oracle subtraction uses ADD_MONTHS with negative values + * and subtracts NUMTODSINTERVAL for time units + */ + public subtractInterval(date: string, interval: string): string { + const intervalParsed = parseSqlInterval(interval); + let res = date; + + // Handle year/month/quarter using ADD_MONTHS with negative values + let totalMonths = 0; + if (intervalParsed.year) { + totalMonths += intervalParsed.year * 12; + } + if (intervalParsed.quarter) { + totalMonths += intervalParsed.quarter * 3; + } + if (intervalParsed.month) { + totalMonths += intervalParsed.month; + } + + if (totalMonths !== 0) { + res = `ADD_MONTHS(${res}, -${totalMonths})`; + } + + // Handle day/hour/minute/second using NUMTODSINTERVAL with subtraction + if (intervalParsed.day) { + res = `${res} - NUMTODSINTERVAL(${intervalParsed.day}, 'DAY')`; + } + if (intervalParsed.hour) { + res = `${res} - NUMTODSINTERVAL(${intervalParsed.hour}, 'HOUR')`; + } + if (intervalParsed.minute) { + res = `${res} - NUMTODSINTERVAL(${intervalParsed.minute}, 'MINUTE')`; + } + if (intervalParsed.second) { + res = `${res} - NUMTODSINTERVAL(${intervalParsed.second}, 'SECOND')`; + } + + return res; + } + public newFilter(filter) { return new OracleFilter(this, filter); } diff --git a/packages/cubejs-schema-compiler/test/unit/oracle-query.test.ts b/packages/cubejs-schema-compiler/test/unit/oracle-query.test.ts index 73aac1e890ccb..e55e4d088c84a 100644 --- a/packages/cubejs-schema-compiler/test/unit/oracle-query.test.ts +++ b/packages/cubejs-schema-compiler/test/unit/oracle-query.test.ts @@ -305,4 +305,50 @@ describe('OracleQuery', () => { expect(sql).toMatch(/GROUP BY.*"visitors"\.source/i); expect(sql).not.toMatch(/GROUP BY\s+\d+/); }); + + it('handles time dimension without granularity in filter', async () => { + await compiler.compile(); + + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: [ + 'visitors.count' + ], + timeDimensions: [{ + dimension: 'visitors.createdAt', + dateRange: ['2020-01-01', '2020-12-31'] + // No granularity specified - used only for filtering + }], + timezone: 'UTC' + }); + + const queryAndParams = query.buildSqlAndParams(); + const sql = queryAndParams[0]; + + // Key test: no GROUP BY on time dimension when granularity is missing + expect(sql).not.toMatch(/GROUP BY.*created_at/i); + }); + + it('uses Oracle-specific interval arithmetic', async () => { + await compiler.compile(); + + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: [ + 'visitors.thisPeriod', + 'visitors.priorPeriod' + ], + timeDimensions: [{ + dimension: 'visitors.createdAt', + granularity: 'year', + dateRange: ['2020-01-01', '2022-12-31'] + }], + timezone: 'UTC' + }); + + const queryAndParams = query.buildSqlAndParams(); + const sql = queryAndParams[0]; + + // Key test: Oracle uses ADD_MONTHS, not PostgreSQL interval syntax + expect(sql).toMatch(/ADD_MONTHS/i); + expect(sql).not.toMatch(/interval '1 year'/i); + }); }); From 1bc22eca4a303de6e1f75fa2c39183e5e6a894db Mon Sep 17 00:00:00 2001 From: Tom Tankilevitch Date: Sun, 19 Oct 2025 19:00:54 +0300 Subject: [PATCH 3/4] add tests for subtractInterval and addInterval in oracle --- .../test/unit/oracle-query.test.ts | 444 ++++++++++++++++++ 1 file changed, 444 insertions(+) diff --git a/packages/cubejs-schema-compiler/test/unit/oracle-query.test.ts b/packages/cubejs-schema-compiler/test/unit/oracle-query.test.ts index e55e4d088c84a..68dd1c8e800d4 100644 --- a/packages/cubejs-schema-compiler/test/unit/oracle-query.test.ts +++ b/packages/cubejs-schema-compiler/test/unit/oracle-query.test.ts @@ -351,4 +351,448 @@ describe('OracleQuery', () => { expect(sql).toMatch(/ADD_MONTHS/i); expect(sql).not.toMatch(/interval '1 year'/i); }); + + describe('addInterval', () => { + it('adds year interval using ADD_MONTHS', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.addInterval('my_date', '1 year'); + expect(result).toBe('ADD_MONTHS(my_date, 12)'); + }); + + it('adds multiple years using ADD_MONTHS', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.addInterval('my_date', '3 year'); + expect(result).toBe('ADD_MONTHS(my_date, 36)'); + }); + + it('adds month interval using ADD_MONTHS', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.addInterval('my_date', '1 month'); + expect(result).toBe('ADD_MONTHS(my_date, 1)'); + }); + + it('adds multiple months using ADD_MONTHS', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.addInterval('my_date', '6 month'); + expect(result).toBe('ADD_MONTHS(my_date, 6)'); + }); + + it('adds quarter interval using ADD_MONTHS', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.addInterval('my_date', '1 quarter'); + expect(result).toBe('ADD_MONTHS(my_date, 3)'); + }); + + it('adds multiple quarters using ADD_MONTHS', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.addInterval('my_date', '4 quarter'); + expect(result).toBe('ADD_MONTHS(my_date, 12)'); + }); + + it('adds day interval using NUMTODSINTERVAL', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.addInterval('my_date', '1 day'); + expect(result).toBe('my_date + NUMTODSINTERVAL(1, \'DAY\')'); + }); + + it('adds multiple days using NUMTODSINTERVAL', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.addInterval('my_date', '7 day'); + expect(result).toBe('my_date + NUMTODSINTERVAL(7, \'DAY\')'); + }); + + it('adds hour interval using NUMTODSINTERVAL', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.addInterval('my_date', '1 hour'); + expect(result).toBe('my_date + NUMTODSINTERVAL(1, \'HOUR\')'); + }); + + it('adds multiple hours using NUMTODSINTERVAL', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.addInterval('my_date', '24 hour'); + expect(result).toBe('my_date + NUMTODSINTERVAL(24, \'HOUR\')'); + }); + + it('adds minute interval using NUMTODSINTERVAL', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.addInterval('my_date', '1 minute'); + expect(result).toBe('my_date + NUMTODSINTERVAL(1, \'MINUTE\')'); + }); + + it('adds multiple minutes using NUMTODSINTERVAL', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.addInterval('my_date', '30 minute'); + expect(result).toBe('my_date + NUMTODSINTERVAL(30, \'MINUTE\')'); + }); + + it('adds second interval using NUMTODSINTERVAL', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.addInterval('my_date', '1 second'); + expect(result).toBe('my_date + NUMTODSINTERVAL(1, \'SECOND\')'); + }); + + it('adds multiple seconds using NUMTODSINTERVAL', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.addInterval('my_date', '45 second'); + expect(result).toBe('my_date + NUMTODSINTERVAL(45, \'SECOND\')'); + }); + + it('combines year and month into single ADD_MONTHS', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.addInterval('my_date', '1 year 6 month'); + expect(result).toBe('ADD_MONTHS(my_date, 18)'); + }); + + it('combines quarter and month into single ADD_MONTHS', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.addInterval('my_date', '2 quarter 3 month'); + expect(result).toBe('ADD_MONTHS(my_date, 9)'); + }); + + it('combines year, quarter, and month into single ADD_MONTHS', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.addInterval('my_date', '2 year 1 quarter 2 month'); + expect(result).toBe('ADD_MONTHS(my_date, 29)'); + }); + + it('combines day and hour intervals', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.addInterval('my_date', '1 day 2 hour'); + expect(result).toBe('my_date + NUMTODSINTERVAL(1, \'DAY\') + NUMTODSINTERVAL(2, \'HOUR\')'); + }); + + it('combines hour, minute, and second intervals', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.addInterval('my_date', '1 hour 30 minute 45 second'); + expect(result).toBe('my_date + NUMTODSINTERVAL(1, \'HOUR\') + NUMTODSINTERVAL(30, \'MINUTE\') + NUMTODSINTERVAL(45, \'SECOND\')'); + }); + + it('combines month-based and day-based intervals', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.addInterval('my_date', '1 year 2 day 3 hour'); + expect(result).toBe('ADD_MONTHS(my_date, 12) + NUMTODSINTERVAL(2, \'DAY\') + NUMTODSINTERVAL(3, \'HOUR\')'); + }); + + it('combines all interval types', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.addInterval('my_date', '1 year 2 quarter 3 month 4 day 5 hour 6 minute 7 second'); + expect(result).toBe('ADD_MONTHS(my_date, 21) + NUMTODSINTERVAL(4, \'DAY\') + NUMTODSINTERVAL(5, \'HOUR\') + NUMTODSINTERVAL(6, \'MINUTE\') + NUMTODSINTERVAL(7, \'SECOND\')'); + }); + + it('handles complex date expressions', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.addInterval('TRUNC(my_date)', '1 month'); + expect(result).toBe('ADD_MONTHS(TRUNC(my_date), 1)'); + }); + }); + + describe('subtractInterval', () => { + it('subtracts year interval using ADD_MONTHS with negative value', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.subtractInterval('my_date', '1 year'); + expect(result).toBe('ADD_MONTHS(my_date, -12)'); + }); + + it('subtracts multiple years using ADD_MONTHS with negative value', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.subtractInterval('my_date', '3 year'); + expect(result).toBe('ADD_MONTHS(my_date, -36)'); + }); + + it('subtracts month interval using ADD_MONTHS with negative value', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.subtractInterval('my_date', '1 month'); + expect(result).toBe('ADD_MONTHS(my_date, -1)'); + }); + + it('subtracts multiple months using ADD_MONTHS with negative value', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.subtractInterval('my_date', '6 month'); + expect(result).toBe('ADD_MONTHS(my_date, -6)'); + }); + + it('subtracts quarter interval using ADD_MONTHS with negative value', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.subtractInterval('my_date', '1 quarter'); + expect(result).toBe('ADD_MONTHS(my_date, -3)'); + }); + + it('subtracts multiple quarters using ADD_MONTHS with negative value', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.subtractInterval('my_date', '4 quarter'); + expect(result).toBe('ADD_MONTHS(my_date, -12)'); + }); + + it('subtracts day interval using NUMTODSINTERVAL subtraction', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.subtractInterval('my_date', '1 day'); + expect(result).toBe('my_date - NUMTODSINTERVAL(1, \'DAY\')'); + }); + + it('subtracts multiple days using NUMTODSINTERVAL subtraction', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.subtractInterval('my_date', '7 day'); + expect(result).toBe('my_date - NUMTODSINTERVAL(7, \'DAY\')'); + }); + + it('subtracts hour interval using NUMTODSINTERVAL subtraction', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.subtractInterval('my_date', '1 hour'); + expect(result).toBe('my_date - NUMTODSINTERVAL(1, \'HOUR\')'); + }); + + it('subtracts multiple hours using NUMTODSINTERVAL subtraction', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.subtractInterval('my_date', '24 hour'); + expect(result).toBe('my_date - NUMTODSINTERVAL(24, \'HOUR\')'); + }); + + it('subtracts minute interval using NUMTODSINTERVAL subtraction', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.subtractInterval('my_date', '1 minute'); + expect(result).toBe('my_date - NUMTODSINTERVAL(1, \'MINUTE\')'); + }); + + it('subtracts multiple minutes using NUMTODSINTERVAL subtraction', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.subtractInterval('my_date', '30 minute'); + expect(result).toBe('my_date - NUMTODSINTERVAL(30, \'MINUTE\')'); + }); + + it('subtracts second interval using NUMTODSINTERVAL subtraction', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.subtractInterval('my_date', '1 second'); + expect(result).toBe('my_date - NUMTODSINTERVAL(1, \'SECOND\')'); + }); + + it('subtracts multiple seconds using NUMTODSINTERVAL subtraction', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.subtractInterval('my_date', '45 second'); + expect(result).toBe('my_date - NUMTODSINTERVAL(45, \'SECOND\')'); + }); + + it('combines year and month into single ADD_MONTHS with negative value', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.subtractInterval('my_date', '1 year 6 month'); + expect(result).toBe('ADD_MONTHS(my_date, -18)'); + }); + + it('combines quarter and month into single ADD_MONTHS with negative value', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.subtractInterval('my_date', '2 quarter 3 month'); + expect(result).toBe('ADD_MONTHS(my_date, -9)'); + }); + + it('combines year, quarter, and month into single ADD_MONTHS with negative value', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.subtractInterval('my_date', '2 year 1 quarter 2 month'); + expect(result).toBe('ADD_MONTHS(my_date, -29)'); + }); + + it('combines day and hour intervals with subtraction', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.subtractInterval('my_date', '1 day 2 hour'); + expect(result).toBe('my_date - NUMTODSINTERVAL(1, \'DAY\') - NUMTODSINTERVAL(2, \'HOUR\')'); + }); + + it('combines hour, minute, and second intervals with subtraction', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.subtractInterval('my_date', '1 hour 30 minute 45 second'); + expect(result).toBe('my_date - NUMTODSINTERVAL(1, \'HOUR\') - NUMTODSINTERVAL(30, \'MINUTE\') - NUMTODSINTERVAL(45, \'SECOND\')'); + }); + + it('combines month-based and day-based intervals with subtraction', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.subtractInterval('my_date', '1 year 2 day 3 hour'); + expect(result).toBe('ADD_MONTHS(my_date, -12) - NUMTODSINTERVAL(2, \'DAY\') - NUMTODSINTERVAL(3, \'HOUR\')'); + }); + + it('combines all interval types with subtraction', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.subtractInterval('my_date', '1 year 2 quarter 3 month 4 day 5 hour 6 minute 7 second'); + expect(result).toBe('ADD_MONTHS(my_date, -21) - NUMTODSINTERVAL(4, \'DAY\') - NUMTODSINTERVAL(5, \'HOUR\') - NUMTODSINTERVAL(6, \'MINUTE\') - NUMTODSINTERVAL(7, \'SECOND\')'); + }); + + it('handles complex date expressions', async () => { + await compiler.compile(); + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: ['visitors.count'], + timezone: 'UTC' + }); + const result = query.subtractInterval('TRUNC(my_date)', '1 month'); + expect(result).toBe('ADD_MONTHS(TRUNC(my_date), -1)'); + }); + }); }); From 60a655249ac1a6441defc69e30c2a69fc65e77c1 Mon Sep 17 00:00:00 2001 From: Tom Tankilevitch Date: Tue, 28 Oct 2025 08:32:55 +0200 Subject: [PATCH 4/4] fix(oracle): exclude time dimensions without granularity from GROUP BY Oracle's custom groupByClause() was incorrectly including time dimensions without granularity in the GROUP BY clause, causing SQL errors when time dimensions were used solely for filtering (no grouping). Changes: - Update OracleQuery.groupByClause() to check selectColumns() before including dimensions in GROUP BY - Time dimensions without granularity return null from selectColumns(), so they are now properly excluded - Add comprehensive tests for both PostgreSQL and Oracle covering: * Time dimensions without granularity (filtering only) * Time dimensions with granularity (grouping and filtering) - Remove unnecessary guard in BaseQuery.dimensionTimeGroupedColumn() as it's no longer needed with the Oracle-specific fix The fix aligns Oracle's behavior with PostgreSQL, which already handles this correctly through its base implementation. --- .../src/adapter/BaseQuery.js | 5 -- .../src/adapter/OracleQuery.ts | 6 +- .../test/unit/oracle-query.test.ts | 30 +++++++++- .../test/unit/postgres-query.test.ts | 58 +++++++++++++++++-- 4 files changed, 87 insertions(+), 12 deletions(-) diff --git a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js index 8e4aeebca4617..c47b0cee83fec 100644 --- a/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js +++ b/packages/cubejs-schema-compiler/src/adapter/BaseQuery.js @@ -3857,11 +3857,6 @@ export class BaseQuery { * @return {string} */ dimensionTimeGroupedColumn(dimension, granularity) { - // Handle case when granularity is not specified (e.g., time dimension used only for filtering) - if (!granularity) { - return this.timeGroupedColumn(null, dimension); - } - let dtDate; // Interval is aligned with natural calendar, so we can use DATE_TRUNC diff --git a/packages/cubejs-schema-compiler/src/adapter/OracleQuery.ts b/packages/cubejs-schema-compiler/src/adapter/OracleQuery.ts index ba17879a381cc..d0417847fe8f2 100644 --- a/packages/cubejs-schema-compiler/src/adapter/OracleQuery.ts +++ b/packages/cubejs-schema-compiler/src/adapter/OracleQuery.ts @@ -56,7 +56,11 @@ export class OracleQuery extends BaseQuery { * using forSelect dimensions for grouping */ public groupByClause() { - const dimensions = this.forSelect().filter((item: any) => !!item.dimension) as BaseDimension[]; + // Only include dimensions that have select columns + // Time dimensions without granularity return null from selectColumns() + const dimensions = this.forSelect().filter((item: any) => ( + !!item.dimension && item.selectColumns && item.selectColumns() + )) as BaseDimension[]; if (!dimensions.length) { return ''; } diff --git a/packages/cubejs-schema-compiler/test/unit/oracle-query.test.ts b/packages/cubejs-schema-compiler/test/unit/oracle-query.test.ts index f287daab1e517..307a5cc5400e5 100644 --- a/packages/cubejs-schema-compiler/test/unit/oracle-query.test.ts +++ b/packages/cubejs-schema-compiler/test/unit/oracle-query.test.ts @@ -324,10 +324,38 @@ describe('OracleQuery', () => { const queryAndParams = query.buildSqlAndParams(); const sql = queryAndParams[0]; - // Key test: no GROUP BY on time dimension when granularity is missing + // Time dimensions without granularity should not appear in GROUP BY expect(sql).not.toMatch(/GROUP BY.*created_at/i); }); + it('handles time dimension with granularity in SELECT and GROUP BY', async () => { + await compiler.compile(); + + const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: [ + 'visitors.count' + ], + timeDimensions: [{ + dimension: 'visitors.createdAt', + granularity: 'day', + dateRange: ['2020-01-01', '2020-12-31'] + }], + timezone: 'UTC' + }); + + const queryAndParams = query.buildSqlAndParams(); + const sql = queryAndParams[0]; + + // Time dimension with granularity should appear in SELECT with TRUNC + expect(sql).toMatch(/TRUNC\(.*created_at/i); + + // Time dimension with granularity should appear in GROUP BY + expect(sql).toMatch(/GROUP BY.*created_at/i); + + // Should still have WHERE clause for filtering + expect(sql).toMatch(/WHERE/i); + }); + it('uses Oracle-specific interval arithmetic', async () => { await compiler.compile(); diff --git a/packages/cubejs-schema-compiler/test/unit/postgres-query.test.ts b/packages/cubejs-schema-compiler/test/unit/postgres-query.test.ts index 1bba2d1ffdd2a..f2a609b743781 100644 --- a/packages/cubejs-schema-compiler/test/unit/postgres-query.test.ts +++ b/packages/cubejs-schema-compiler/test/unit/postgres-query.test.ts @@ -329,6 +329,59 @@ describe('PostgresQuery', () => { }); }); + it('handles time dimension without granularity in filter', async () => { + await compiler.compile(); + + const query = new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: [ + 'visitors.count' + ], + timeDimensions: [{ + dimension: 'visitors.createdAt', + dateRange: ['2020-01-01', '2020-12-31'] + // No granularity specified - used only for filtering + }], + timezone: 'UTC' + }); + + const queryAndParams = query.buildSqlAndParams(); + const sql = queryAndParams[0]; + + // Time dimensions without granularity should not appear in GROUP BY + expect(sql).not.toMatch(/GROUP BY.*created_at/i); + + // Time dimension should still be used in WHERE clause for filtering + expect(sql).toMatch(/WHERE/i); + }); + + it('handles time dimension with granularity in SELECT and GROUP BY', async () => { + await compiler.compile(); + + const query = new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, { + measures: [ + 'visitors.count' + ], + timeDimensions: [{ + dimension: 'visitors.createdAt', + granularity: 'day', + dateRange: ['2020-01-01', '2020-12-31'] + }], + timezone: 'UTC' + }); + + const queryAndParams = query.buildSqlAndParams(); + const sql = queryAndParams[0]; + + // Time dimension with granularity should appear in SELECT + expect(sql).toMatch(/date_trunc\('day',.*created_at/i); + + // Time dimension with granularity should appear in GROUP BY + expect(sql).toMatch(/GROUP BY/i); + + // Should still have WHERE clause for filtering + expect(sql).toMatch(/WHERE/i); + }); + it('uses AS keyword in subquery aliases (regression test)', async () => { await compiler.compile(); @@ -350,10 +403,5 @@ describe('PostgresQuery', () => { // PostgreSQL should use AS keyword for subquery aliases expect(sql).toMatch(/\s+AS\s+q_0\s+/); - - // Should NOT have pattern ) q_0 (without AS) - // This regex checks for closing paren followed by space(s), q_0, space, but NOT preceded by AS - const hasAsKeyword = /\s+AS\s+q_0\s+/.test(sql); - expect(hasAsKeyword).toBe(true); }); });