Skip to content

Commit cafc7fb

Browse files
committed
fix(oracle): resolve bind parameter mismatch for generated time series
Refactor Oracle date/time casting logic to properly handle column identifiers from generated time series (date_from/date_to) without treating them as bind parameters. Previously, all values passed to dateTimeCast/timeStampCast were treated as bind parameters, causing Oracle NJS-098 errors when queries with rolling windows generated 10 bind placeholders but only 6 parameter values. Changes: - Add helper methods isIdentifierToken() and toTimestampTz() to centralize casting logic - Detect column identifiers vs bind parameters ('?') - Use direct column references for generated series columns - Preserve bind parameters for filter values only - Add comprehensive JSDoc documentation Fixes bind parameter count mismatch that caused NJS-098 errors.
1 parent b0b011e commit cafc7fb

File tree

2 files changed

+120
-5
lines changed

2 files changed

+120
-5
lines changed

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

Lines changed: 56 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,46 @@ class OracleFilter extends BaseFilter {
3030
}
3131

3232
export class OracleQuery extends BaseQuery {
33+
private static readonly ORACLE_TZ_FORMAT_WITH_Z = 'YYYY-MM-DD"T"HH24:MI:SS.FF"Z"';
34+
35+
private static readonly ORACLE_TZ_FORMAT_NO_Z = 'YYYY-MM-DD"T"HH24:MI:SS.FF';
36+
37+
/**
38+
* Determines if a value represents a SQL identifier (column name) rather than a bind parameter.
39+
* Handles both unquoted identifiers (e.g., "date_from", "table.column") and quoted
40+
* identifiers (e.g., "date_from", "table"."column").
41+
*/
42+
private isIdentifierToken(value: string): boolean {
43+
return (
44+
/^[A-Za-z_][A-Za-z0-9_]*(\.[A-Za-z_][A-Za-z0-9_]*)*$/.test(value) ||
45+
/^"[^"]+"(\."[^"]+")*$/.test(value)
46+
);
47+
}
48+
49+
/**
50+
* Generates Oracle TO_TIMESTAMP_TZ function call for timezone-aware timestamp conversion.
51+
*
52+
* The format string must match the actual data format:
53+
* - Filter parameters ('?') come as ISO 8601 strings with 'Z' suffix (e.g., '2024-01-01T00:00:00.000Z')
54+
* - Generated time series columns (date_from/date_to) contain VALUES data without 'Z' (e.g., '2024-01-01T00:00:00.000')
55+
*
56+
* @param value - Either '?' for bind parameters, a column identifier, or a SQL expression
57+
* @param includeZFormat - Whether format string should expect 'Z' suffix (true for filter params, false for series columns)
58+
* @returns Oracle SQL expression with appropriate bind placeholder or direct column reference
59+
*/
60+
private toTimestampTz(value: string, includeZFormat: boolean): string {
61+
const format = includeZFormat ? OracleQuery.ORACLE_TZ_FORMAT_WITH_Z : OracleQuery.ORACLE_TZ_FORMAT_NO_Z;
62+
if (value === '?') {
63+
return `TO_TIMESTAMP_TZ(:"?", '${format}')`;
64+
}
65+
if (this.isIdentifierToken(value)) {
66+
// Column identifiers (e.g., date_from, date_to from generated time series) - use directly
67+
return `TO_TIMESTAMP_TZ(${value}, '${format}')`;
68+
}
69+
// SQL expressions or literals - embed directly in TO_TIMESTAMP_TZ call
70+
return `TO_TIMESTAMP_TZ(${value}, '${format}')`;
71+
}
72+
3373
/**
3474
* "LIMIT" on Oracle is illegal
3575
* TODO replace with limitOffsetClause override
@@ -75,15 +115,26 @@ export class OracleQuery extends BaseQuery {
75115
return field;
76116
}
77117

118+
/**
119+
* Casts a value to Oracle DATE type using timezone-aware parsing.
120+
* For bind parameters ('?'), includes 'Z' suffix in format string.
121+
* For column identifiers (e.g., date_from/date_to from time series), omits 'Z'.
122+
*
123+
* @param value - Bind parameter placeholder '?', column identifier, or SQL expression
124+
*/
78125
public dateTimeCast(value) {
79-
// Use timezone-aware parsing for ISO 8601 with milliseconds and trailing 'Z', then cast to DATE
80-
// to preserve index-friendly comparisons against DATE columns.
81-
return `CAST(TO_TIMESTAMP_TZ(:"${value}", 'YYYY-MM-DD"T"HH24:MI:SS.FF"Z"') AS DATE)`;
126+
return `CAST(${this.toTimestampTz(value, value === '?')} AS DATE)`;
82127
}
83128

129+
/**
130+
* Casts a value to Oracle TIMESTAMP WITH TIME ZONE.
131+
* For bind parameters ('?'), includes 'Z' suffix in format string.
132+
* For column identifiers (e.g., date_from/date_to from time series), omits 'Z'.
133+
*
134+
* @param value - Bind parameter placeholder '?', column identifier, or SQL expression
135+
*/
84136
public timeStampCast(value) {
85-
// Return timezone-aware timestamp for TIMESTAMP comparisons
86-
return `TO_TIMESTAMP_TZ(:"${value}", 'YYYY-MM-DD"T"HH24:MI:SS.FF"Z"')`;
137+
return this.toTimestampTz(value, value === '?');
87138
}
88139

89140
public timeStampParam(timeDimension) {

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

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -904,4 +904,68 @@ describe('OracleQuery', () => {
904904
expect(sql).toMatch(/GROUP BY\s+TRUNC/);
905905
expect(params).toEqual(['2024-01-01T00:00:00.000Z', '2024-12-31T23:59:59.999Z']);
906906
});
907+
908+
it('does not bind generated time series date_from/date_to', async () => {
909+
await compiler.compile();
910+
911+
const query = new OracleQuery(
912+
{ joinGraph, cubeEvaluator, compiler },
913+
{
914+
measures: ['visitors.thisPeriod', 'visitors.priorPeriod'],
915+
timeDimensions: [
916+
{
917+
dimension: 'visitors.createdAt',
918+
dateRange: ['2023-01-01', '2024-12-31'],
919+
granularity: 'year'
920+
}
921+
],
922+
filters: [
923+
{ member: 'visitors.source', operator: 'equals', values: ['web'] }
924+
],
925+
timezone: 'UTC'
926+
}
927+
);
928+
929+
const [sql] = query.buildSqlAndParams();
930+
931+
// Ensure generated time series columns are not treated as bind params
932+
expect(sql).not.toMatch(/:\s*"date_from"/);
933+
expect(sql).not.toMatch(/:\s*"date_to"/);
934+
935+
// Ensure we cast series columns directly via TO_TIMESTAMP_TZ(..., '...FF') without binds
936+
expect(sql).toMatch(/TO_TIMESTAMP_TZ\(date_from,\s*'YYYY-MM-DD"T"HH24:MI:SS\.FF'\)/);
937+
expect(sql).toMatch(/TO_TIMESTAMP_TZ\(date_to,\s*'YYYY-MM-DD"T"HH24:MI:SS\.FF'\)/);
938+
});
939+
940+
it('uses Z format for bind parameters and no-Z format for column identifiers', async () => {
941+
await compiler.compile();
942+
943+
const query = new OracleQuery(
944+
{ joinGraph, cubeEvaluator, compiler },
945+
{
946+
measures: ['visitors.count'],
947+
timezone: 'UTC'
948+
}
949+
);
950+
951+
// Test direct method calls to verify format selection logic
952+
// Bind parameter '?' should use Z format (for ISO 8601 strings with Z)
953+
const bindResult = query.dateTimeCast('?');
954+
expect(bindResult).toContain('TO_TIMESTAMP_TZ(:"?",');
955+
expect(bindResult).toContain('YYYY-MM-DD"T"HH24:MI:SS.FF"Z"');
956+
957+
// Column identifier should use no-Z format (for VALUES data without Z)
958+
const columnResult = query.dateTimeCast('date_from');
959+
expect(columnResult).toContain('TO_TIMESTAMP_TZ(date_from,');
960+
expect(columnResult).toContain('YYYY-MM-DD"T"HH24:MI:SS.FF');
961+
expect(columnResult).not.toContain('"Z"');
962+
963+
// Verify timeStampCast has same behavior
964+
const bindTimestamp = query.timeStampCast('?');
965+
expect(bindTimestamp).toContain('YYYY-MM-DD"T"HH24:MI:SS.FF"Z"');
966+
967+
const columnTimestamp = query.timeStampCast('date_to');
968+
expect(columnTimestamp).toContain('YYYY-MM-DD"T"HH24:MI:SS.FF');
969+
expect(columnTimestamp).not.toContain('"Z"');
970+
});
907971
});

0 commit comments

Comments
 (0)