Skip to content

Commit ea3ba21

Browse files
committed
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 ...
1 parent 644fbd4 commit ea3ba21

File tree

3 files changed

+339
-3
lines changed

3 files changed

+339
-3
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) {
Lines changed: 308 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,308 @@
1+
/* eslint-disable no-restricted-syntax */
2+
import { OracleQuery } from '../../src/adapter/OracleQuery';
3+
import { prepareJsCompiler } from './PrepareCompiler';
4+
5+
describe('OracleQuery', () => {
6+
const { compiler, joinGraph, cubeEvaluator } = prepareJsCompiler(`
7+
cube(\`visitors\`, {
8+
sql: \`
9+
select * from visitors
10+
\`,
11+
12+
measures: {
13+
count: {
14+
type: 'count'
15+
},
16+
17+
unboundedCount: {
18+
type: 'count',
19+
rollingWindow: {
20+
trailing: 'unbounded'
21+
}
22+
},
23+
24+
thisPeriod: {
25+
sql: 'amount',
26+
type: 'sum',
27+
rollingWindow: {
28+
trailing: '1 year',
29+
offset: 'end'
30+
}
31+
},
32+
33+
priorPeriod: {
34+
sql: 'amount',
35+
type: 'sum',
36+
rollingWindow: {
37+
trailing: '1 year',
38+
offset: 'start'
39+
}
40+
}
41+
},
42+
43+
dimensions: {
44+
id: {
45+
sql: 'id',
46+
type: 'number',
47+
primaryKey: true
48+
},
49+
50+
createdAt: {
51+
type: 'time',
52+
sql: 'created_at'
53+
},
54+
55+
source: {
56+
type: 'string',
57+
sql: 'source'
58+
}
59+
}
60+
})
61+
62+
cube(\`Deals\`, {
63+
sql: \`select * from deals\`,
64+
65+
measures: {
66+
amount: {
67+
sql: \`amount\`,
68+
type: \`sum\`
69+
}
70+
},
71+
72+
dimensions: {
73+
salesManagerId: {
74+
sql: \`sales_manager_id\`,
75+
type: 'string',
76+
primaryKey: true
77+
}
78+
}
79+
})
80+
81+
cube(\`SalesManagers\`, {
82+
sql: \`select * from sales_managers\`,
83+
84+
joins: {
85+
Deals: {
86+
relationship: \`hasMany\`,
87+
sql: \`\${SalesManagers}.id = \${Deals}.sales_manager_id\`
88+
}
89+
},
90+
91+
measures: {
92+
averageDealAmount: {
93+
sql: \`\${dealsAmount}\`,
94+
type: \`avg\`
95+
}
96+
},
97+
98+
dimensions: {
99+
id: {
100+
sql: \`id\`,
101+
type: \`string\`,
102+
primaryKey: true
103+
},
104+
105+
dealsAmount: {
106+
sql: \`\${Deals.amount}\`,
107+
type: \`number\`,
108+
subQuery: true
109+
}
110+
}
111+
});
112+
`, { adapter: 'oracle' });
113+
114+
it('basic query without subqueries', async () => {
115+
await compiler.compile();
116+
117+
const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, {
118+
measures: [
119+
'visitors.count'
120+
],
121+
timeDimensions: [],
122+
timezone: 'UTC'
123+
});
124+
125+
const queryAndParams = query.buildSqlAndParams();
126+
const sql = queryAndParams[0];
127+
128+
// Basic query should work
129+
expect(sql).toContain('SELECT');
130+
expect(sql).toMatch(/FROM\s+visitors/i);
131+
// Should not have subquery aliases in simple query
132+
expect(sql).not.toMatch(/\bq_\d+\b/);
133+
// Should use Oracle FETCH NEXT
134+
expect(sql).toContain('FETCH NEXT');
135+
});
136+
137+
it('does not use AS keyword in subquery aliases with single rolling window', async () => {
138+
await compiler.compile();
139+
140+
const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, {
141+
measures: [
142+
'visitors.count',
143+
'visitors.unboundedCount'
144+
],
145+
timeDimensions: [{
146+
dimension: 'visitors.createdAt',
147+
granularity: 'day',
148+
dateRange: ['2020-01-01', '2020-01-31']
149+
}],
150+
timezone: 'UTC'
151+
});
152+
153+
const queryAndParams = query.buildSqlAndParams();
154+
const sql = queryAndParams[0];
155+
156+
// Oracle should NOT have AS keyword before subquery aliases
157+
expect(sql).not.toMatch(/\bAS\s+q_\d+/i);
158+
expect(sql).not.toMatch(/\bas\s+q_\d+/);
159+
160+
// Should have q_0 alias (with space around it, indicating no AS)
161+
expect(sql).toMatch(/\)\s+q_0\s+/);
162+
});
163+
164+
it('does not use AS keyword with multiple rolling window measures (YoY scenario)', async () => {
165+
await compiler.compile();
166+
167+
const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, {
168+
measures: [
169+
'visitors.thisPeriod',
170+
'visitors.priorPeriod'
171+
],
172+
timeDimensions: [{
173+
dimension: 'visitors.createdAt',
174+
granularity: 'year',
175+
dateRange: ['2020-01-01', '2022-12-31']
176+
}],
177+
timezone: 'UTC'
178+
});
179+
180+
const queryAndParams = query.buildSqlAndParams();
181+
const sql = queryAndParams[0];
182+
183+
// Should have multiple subquery aliases (q_0, q_1, q_2, etc.)
184+
expect(sql).toMatch(/\bq_0\b/);
185+
expect(sql).toMatch(/\bq_1\b/);
186+
187+
// Oracle should NOT have AS keyword anywhere before q_ aliases
188+
expect(sql).not.toMatch(/\bAS\s+q_\d+/i);
189+
expect(sql).not.toMatch(/\bas\s+q_\d+/);
190+
191+
// Verify pattern is ) q_X not ) AS q_X
192+
expect(sql).toMatch(/\)\s+q_\d+/);
193+
});
194+
195+
it('does not use AS keyword in INNER JOIN subqueries', async () => {
196+
await compiler.compile();
197+
198+
const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, {
199+
dimensions: [
200+
'SalesManagers.id',
201+
'SalesManagers.dealsAmount'
202+
]
203+
});
204+
205+
const queryAndParams = query.buildSqlAndParams();
206+
const sql = queryAndParams[0];
207+
208+
// Should have INNER JOIN for subquery dimension
209+
if (sql.includes('INNER JOIN')) {
210+
// Oracle should NOT have AS keyword in INNER JOIN
211+
expect(sql).not.toMatch(/INNER\s+JOIN\s+\([^)]+\)\s+AS\s+/i);
212+
expect(sql).not.toMatch(/INNER\s+JOIN\s+\([^)]+\)\s+as\s+/);
213+
}
214+
});
215+
216+
it('uses FETCH NEXT syntax instead of LIMIT', async () => {
217+
await compiler.compile();
218+
219+
const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, {
220+
measures: [
221+
'visitors.count'
222+
],
223+
timezone: 'UTC',
224+
limit: 100
225+
});
226+
227+
const queryAndParams = query.buildSqlAndParams();
228+
const sql = queryAndParams[0];
229+
230+
// Oracle should use FETCH NEXT instead of LIMIT
231+
expect(sql).toContain('FETCH NEXT');
232+
expect(sql).toContain('ROWS ONLY');
233+
expect(sql).not.toContain('LIMIT');
234+
});
235+
236+
it('uses FETCH NEXT syntax with subqueries and rolling windows', async () => {
237+
await compiler.compile();
238+
239+
const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, {
240+
measures: [
241+
'visitors.thisPeriod',
242+
'visitors.priorPeriod'
243+
],
244+
timeDimensions: [{
245+
dimension: 'visitors.createdAt',
246+
granularity: 'month',
247+
dateRange: ['2020-01-01', '2020-12-31']
248+
}],
249+
timezone: 'UTC',
250+
limit: 50
251+
});
252+
253+
const queryAndParams = query.buildSqlAndParams();
254+
const sql = queryAndParams[0];
255+
256+
// Should have subqueries without AS
257+
expect(sql).not.toMatch(/\bAS\s+q_\d+/i);
258+
259+
// Should use Oracle-specific FETCH NEXT
260+
expect(sql).toContain('FETCH NEXT');
261+
expect(sql).toContain('ROWS ONLY');
262+
expect(sql).not.toContain('LIMIT');
263+
});
264+
265+
it('does not use AS keyword with comma-separated subqueries', async () => {
266+
await compiler.compile();
267+
268+
const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, {
269+
measures: [
270+
'visitors.thisPeriod',
271+
'visitors.priorPeriod'
272+
],
273+
timezone: 'UTC'
274+
});
275+
276+
const queryAndParams = query.buildSqlAndParams();
277+
const sql = queryAndParams[0];
278+
279+
// Should have multiple subquery aliases
280+
expect(sql).toMatch(/\)\s+q_0\s+,/);
281+
expect(sql).toMatch(/\)\s+q_1\s+/);
282+
283+
// Should NOT have AS before q_ aliases
284+
expect(sql).not.toMatch(/\bAS\s+q_\d+/i);
285+
expect(sql).not.toMatch(/\bas\s+q_\d+/);
286+
});
287+
288+
it('group by dimensions not indexes', async () => {
289+
await compiler.compile();
290+
291+
const query = new OracleQuery({ joinGraph, cubeEvaluator, compiler }, {
292+
measures: [
293+
'visitors.count'
294+
],
295+
dimensions: [
296+
'visitors.source'
297+
],
298+
timezone: 'UTC'
299+
});
300+
301+
const queryAndParams = query.buildSqlAndParams();
302+
const sql = queryAndParams[0];
303+
304+
// Oracle should group by actual dimension SQL, not by index
305+
expect(sql).toMatch(/GROUP BY.*"visitors"\.source/i);
306+
expect(sql).not.toMatch(/GROUP BY\s+\d+/);
307+
});
308+
});

packages/cubejs-schema-compiler/test/unit/postgres-query.test.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -328,4 +328,32 @@ describe('PostgresQuery', () => {
328328
expect(queryAndParams[0]).toContain('ORDER BY 3 ASC');
329329
});
330330
});
331+
332+
it('uses AS keyword in subquery aliases (regression test)', async () => {
333+
await compiler.compile();
334+
335+
const query = new PostgresQuery({ joinGraph, cubeEvaluator, compiler }, {
336+
measures: [
337+
'visitors.count',
338+
'visitors.unboundedCount'
339+
],
340+
timeDimensions: [{
341+
dimension: 'visitors.createdAt',
342+
granularity: 'day',
343+
dateRange: ['2020-01-01', '2020-01-31']
344+
}],
345+
timezone: 'UTC'
346+
});
347+
348+
const queryAndParams = query.buildSqlAndParams();
349+
const sql = queryAndParams[0];
350+
351+
// PostgreSQL should use AS keyword for subquery aliases
352+
expect(sql).toMatch(/\s+AS\s+q_0\s+/);
353+
354+
// Should NOT have pattern ) q_0 (without AS)
355+
// This regex checks for closing paren followed by space(s), q_0, space, but NOT preceded by AS
356+
const hasAsKeyword = /\s+AS\s+q_0\s+/.test(sql);
357+
expect(hasAsKeyword).toBe(true);
358+
});
331359
});

0 commit comments

Comments
 (0)