Skip to content

Commit ed791ed

Browse files
Tankilevitchigorlukanin
Tankilevitch
authored and
igorlukanin
committed
fix(schema-compiler): add support for time filters and rolling windows and fix subquery aliasing in Oracle dialect (#10066)
* 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 ... * 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. * add tests for subtractInterval and addInterval in oracle * 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.
1 parent 32a1db6 commit ed791ed

File tree

4 files changed

+964
-5
lines changed

4 files changed

+964
-5
lines changed

‎packages/cubejs-schema-compiler/src/adapter/BaseQuery.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1380,8 +1380,8 @@ export class BaseQuery {
13801380
const join = R.drop(1, toJoin)
13811381
.map(
13821382
(q, i) => (this.dimensionAliasNames().length ?
1383-
`INNER JOIN ${this.wrapInParenthesis((q))} as q_${i + 1} ON ${this.dimensionsJoinCondition(`q_${i}`, `q_${i + 1}`)}` :
1384-
`, ${this.wrapInParenthesis(q)} as q_${i + 1}`),
1383+
`INNER JOIN ${this.wrapInParenthesis((q))} ${this.asSyntaxJoin} q_${i + 1} ON ${this.dimensionsJoinCondition(`q_${i}`, `q_${i + 1}`)}` :
1384+
`, ${this.wrapInParenthesis(q)} ${this.asSyntaxJoin} q_${i + 1}`),
13851385
).join('\n');
13861386

13871387
const columnsToSelect = this.evaluateSymbolSqlWithContext(
@@ -1410,7 +1410,7 @@ export class BaseQuery {
14101410
return `${toJoin[0].replace(/^SELECT/, `SELECT ${this.topLimit()}`)} ${this.orderBy()}${this.groupByDimensionLimit()}`;
14111411
}
14121412

1413-
return `SELECT ${this.topLimit()}${columnsToSelect} FROM ${this.wrapInParenthesis(toJoin[0])} as q_0 ${join}${havingFilters}${this.orderBy()}${this.groupByDimensionLimit()}`;
1413+
return `SELECT ${this.topLimit()}${columnsToSelect} FROM ${this.wrapInParenthesis(toJoin[0])} ${this.asSyntaxJoin} q_0 ${join}${havingFilters}${this.orderBy()}${this.groupByDimensionLimit()}`;
14141414
}
14151415

14161416
wrapInParenthesis(select) {

‎packages/cubejs-schema-compiler/src/adapter/OracleQuery.ts‎

Lines changed: 89 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
1+
import { parseSqlInterval } from '@cubejs-backend/shared';
12
import { BaseQuery } from './BaseQuery';
23
import { BaseFilter } from './BaseFilter';
34
import { UserError } from '../compiler/UserError';
4-
import { BaseDimension } from './BaseDimension';
5+
import type { BaseDimension } from './BaseDimension';
56

67
const GRANULARITY_VALUE = {
78
day: 'DD',
@@ -55,7 +56,11 @@ export class OracleQuery extends BaseQuery {
5556
* using forSelect dimensions for grouping
5657
*/
5758
public groupByClause() {
58-
const dimensions = this.forSelect().filter((item: any) => !!item.dimension) as BaseDimension[];
59+
// Only include dimensions that have select columns
60+
// Time dimensions without granularity return null from selectColumns()
61+
const dimensions = this.forSelect().filter((item: any) => (
62+
!!item.dimension && item.selectColumns && item.selectColumns()
63+
)) as BaseDimension[];
5964
if (!dimensions.length) {
6065
return '';
6166
}
@@ -92,6 +97,88 @@ export class OracleQuery extends BaseQuery {
9297
return `TRUNC(${dimension}, '${GRANULARITY_VALUE[granularity]}')`;
9398
}
9499

100+
/**
101+
* Oracle uses ADD_MONTHS for year/month/quarter intervals
102+
* and NUMTODSINTERVAL for day/hour/minute/second intervals
103+
*/
104+
public addInterval(date: string, interval: string): string {
105+
const intervalParsed = parseSqlInterval(interval);
106+
let res = date;
107+
108+
// Handle year/month/quarter using ADD_MONTHS
109+
let totalMonths = 0;
110+
if (intervalParsed.year) {
111+
totalMonths += intervalParsed.year * 12;
112+
}
113+
if (intervalParsed.quarter) {
114+
totalMonths += intervalParsed.quarter * 3;
115+
}
116+
if (intervalParsed.month) {
117+
totalMonths += intervalParsed.month;
118+
}
119+
120+
if (totalMonths !== 0) {
121+
res = `ADD_MONTHS(${res}, ${totalMonths})`;
122+
}
123+
124+
// Handle day/hour/minute/second using NUMTODSINTERVAL
125+
if (intervalParsed.day) {
126+
res = `${res} + NUMTODSINTERVAL(${intervalParsed.day}, 'DAY')`;
127+
}
128+
if (intervalParsed.hour) {
129+
res = `${res} + NUMTODSINTERVAL(${intervalParsed.hour}, 'HOUR')`;
130+
}
131+
if (intervalParsed.minute) {
132+
res = `${res} + NUMTODSINTERVAL(${intervalParsed.minute}, 'MINUTE')`;
133+
}
134+
if (intervalParsed.second) {
135+
res = `${res} + NUMTODSINTERVAL(${intervalParsed.second}, 'SECOND')`;
136+
}
137+
138+
return res;
139+
}
140+
141+
/**
142+
* Oracle subtraction uses ADD_MONTHS with negative values
143+
* and subtracts NUMTODSINTERVAL for time units
144+
*/
145+
public subtractInterval(date: string, interval: string): string {
146+
const intervalParsed = parseSqlInterval(interval);
147+
let res = date;
148+
149+
// Handle year/month/quarter using ADD_MONTHS with negative values
150+
let totalMonths = 0;
151+
if (intervalParsed.year) {
152+
totalMonths += intervalParsed.year * 12;
153+
}
154+
if (intervalParsed.quarter) {
155+
totalMonths += intervalParsed.quarter * 3;
156+
}
157+
if (intervalParsed.month) {
158+
totalMonths += intervalParsed.month;
159+
}
160+
161+
if (totalMonths !== 0) {
162+
res = `ADD_MONTHS(${res}, -${totalMonths})`;
163+
}
164+
165+
// Handle day/hour/minute/second using NUMTODSINTERVAL with subtraction
166+
if (intervalParsed.day) {
167+
res = `${res} - NUMTODSINTERVAL(${intervalParsed.day}, 'DAY')`;
168+
}
169+
if (intervalParsed.hour) {
170+
res = `${res} - NUMTODSINTERVAL(${intervalParsed.hour}, 'HOUR')`;
171+
}
172+
if (intervalParsed.minute) {
173+
res = `${res} - NUMTODSINTERVAL(${intervalParsed.minute}, 'MINUTE')`;
174+
}
175+
if (intervalParsed.second) {
176+
res = `${res} - NUMTODSINTERVAL(${intervalParsed.second}, 'SECOND')`;
177+
}
178+
179+
return res;
180+
}
181+
95182
public newFilter(filter) {
96183
return new OracleFilter(this, filter);
97184
}

0 commit comments

Comments
 (0)