Skip to content

Commit 45cacb6

Browse files
committed
fix(BigqueryQuery): Prevent double-wrapping of TIMESTAMP() in SQL generation
Updated the SQL generation logic to ensure that TIMESTAMP() is not wrapped in DATETIME() for BigQuery queries. Added regression tests to verify the correct behavior in both standard and SQL pushdown contexts.
1 parent f1621e4 commit 45cacb6

File tree

3 files changed

+121
-2
lines changed

3 files changed

+121
-2
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ export class BigqueryQuery extends BaseQuery {
4242
}
4343

4444
public convertTz(field) {
45-
return `DATETIME(${this.timeStampCast(field)}, '${this.timezone}')`;
45+
return `DATETIME(${field}, '${this.timezone}')`;
4646
}
4747

4848
public timeStampCast(value) {
@@ -263,7 +263,7 @@ export class BigqueryQuery extends BaseQuery {
263263
templates.expressions.binary = '{% if op == \'%\' %}MOD({{ left }}, {{ right }}){% else %}({{ left }} {{ op }} {{ right }}){% endif %}';
264264
templates.expressions.interval = 'INTERVAL {{ interval }}';
265265
templates.expressions.extract = 'EXTRACT({% if date_part == \'DOW\' %}DAYOFWEEK{% elif date_part == \'DOY\' %}DAYOFYEAR{% else %}{{ date_part }}{% endif %} FROM {{ expr }})';
266-
templates.expressions.timestamp_literal = 'DATETIME(TIMESTAMP(\'{{ value }}\'))';
266+
templates.expressions.timestamp_literal = 'TIMESTAMP(\'{{ value }}\')';
267267
delete templates.expressions.ilike;
268268
delete templates.expressions.like_escape;
269269
templates.filters.like_pattern = 'CONCAT({% if start_wild %}\'%\'{% else %}\'\'{% endif %}, LOWER({{ value }}), {% if end_wild %}\'%\'{% else %}\'\'{% endif %})';

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

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1012,6 +1012,73 @@ describe('SQL Generation', () => {
10121012
})
10131013
);
10141014

1015+
it('BigQuery timestamp literal regression test - should not double-wrap DATETIME(TIMESTAMP())', async () => {
1016+
await compilers.compiler.compile();
1017+
1018+
const query = new BigqueryQuery(compilers, {
1019+
measures: [
1020+
'cards.count'
1021+
],
1022+
timeDimensions: [{
1023+
dimension: 'cards.createdAt',
1024+
granularity: 'day',
1025+
dateRange: ['2025-06-03', '2025-06-10']
1026+
}],
1027+
filters: [],
1028+
timezone: 'UTC'
1029+
});
1030+
1031+
const queryAndParams = query.buildSqlAndParams();
1032+
const sql = queryAndParams[0];
1033+
1034+
// Should contain TIMESTAMP() but NOT DATETIME(TIMESTAMP())
1035+
expect(sql).toContain('TIMESTAMP(');
1036+
expect(sql).not.toContain('DATETIME(TIMESTAMP(');
1037+
// Should contain correct BigQuery timestamp comparison syntax with parameters
1038+
expect(sql).toContain('TIMESTAMP(?)');
1039+
// Should contain the properly formatted DATETIME function without double wrapping
1040+
expect(sql).toContain('DATETIME(`cards`.created_at, \'UTC\')');
1041+
});
1042+
1043+
it('BigQuery SQL pushdown timestamp literal test - should not double-wrap DATETIME(TIMESTAMP())', async () => {
1044+
await compilers.compiler.compile();
1045+
1046+
const query = new BigqueryQuery(compilers, {
1047+
measures: [
1048+
'cards.count'
1049+
],
1050+
timeDimensions: [{
1051+
dimension: 'cards.createdAt',
1052+
granularity: 'day',
1053+
dateRange: ['2025-06-03', '2025-06-10']
1054+
}],
1055+
filters: [{
1056+
member: 'cards.createdAt',
1057+
operator: 'inDateRange',
1058+
values: ['2025-06-03', '2025-06-10']
1059+
}],
1060+
timezone: 'UTC',
1061+
// Simulate SQL pushdown scenario with external query context
1062+
ungrouped: true
1063+
});
1064+
1065+
const queryAndParams = query.buildSqlAndParams();
1066+
const sql = queryAndParams[0];
1067+
1068+
console.log('Generated SQL pushdown:', sql);
1069+
1070+
// Should contain TIMESTAMP() but NOT DATETIME(TIMESTAMP()) in pushdown context
1071+
expect(sql).toContain('TIMESTAMP(');
1072+
expect(sql).not.toContain('DATETIME(TIMESTAMP(');
1073+
// Should contain correct BigQuery timestamp comparison syntax with parameters
1074+
expect(sql).toContain('TIMESTAMP(?)');
1075+
// Should contain the properly formatted DATETIME function without double wrapping
1076+
expect(sql).toContain('DATETIME(`cards`.created_at, \'UTC\')');
1077+
// Should handle date range filters correctly in pushdown context
1078+
expect(sql).toMatch(/DATETIME\(`cards`\.created_at, 'UTC'\) >= TIMESTAMP\(\?\)/);
1079+
expect(sql).toMatch(/DATETIME\(`cards`\.created_at, 'UTC'\) <= TIMESTAMP\(\?\)/);
1080+
});
1081+
10151082
it('Test time series with 6 digits timestamp precision - bigquery', async () => {
10161083
await compilers.compiler.compile();
10171084

rust/cubesql/cubesql/src/compile/mod.rs

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16970,4 +16970,56 @@ LIMIT {{ limit }}{% endif %}"#.to_string(),
1697016970
displayable(physical_plan.as_ref()).indent()
1697116971
);
1697216972
}
16973+
16974+
#[tokio::test]
16975+
async fn test_bigquery_timestamp_literal_regression() {
16976+
if !Rewriter::sql_push_down_enabled() {
16977+
return;
16978+
}
16979+
init_testing_logger();
16980+
16981+
let bq_templates = vec![
16982+
("expressions/timestamp_literal".to_string(), "TIMESTAMP('{{ value }}')".to_string()),
16983+
];
16984+
16985+
let query_plan = convert_select_to_query_plan_customized(
16986+
r#"
16987+
SELECT COUNT(*) AS count_orders, LOWER(customer_gender) AS gender
16988+
FROM KibanaSampleDataEcommerce
16989+
WHERE order_date >= TIMESTAMP '2025-06-03 00:00:00'
16990+
AND order_date <= TIMESTAMP '2025-06-10 23:59:59'
16991+
AND LOWER(customer_gender) = 'test'
16992+
GROUP BY LOWER(customer_gender)
16993+
ORDER BY 1 DESC
16994+
"#
16995+
.to_string(),
16996+
DatabaseProtocol::PostgreSQL,
16997+
bq_templates,
16998+
)
16999+
.await;
17000+
17001+
let logical_plan = query_plan.as_logical_plan();
17002+
let sql = logical_plan.find_cube_scan_wrapped_sql().wrapped_sql.sql;
17003+
17004+
println!("Generated BigQuery SQL: {}", sql);
17005+
17006+
// Should contain TIMESTAMP() but NOT DATETIME(TIMESTAMP())
17007+
assert!(sql.contains("TIMESTAMP("));
17008+
assert!(!sql.contains("DATETIME(TIMESTAMP("));
17009+
17010+
// Should contain correct BigQuery timestamp literal syntax
17011+
assert!(sql.contains("TIMESTAMP('2025-06-03T00:00:00.000Z')"));
17012+
assert!(sql.contains("TIMESTAMP('2025-06-10T23:59:59.000Z')"));
17013+
17014+
// The key test: ensure we don't have the double-wrapping issue
17015+
// In the broken version, BigQuery would generate DATETIME(TIMESTAMP()) instead of just TIMESTAMP()
17016+
// Our fix ensures timestamp literals are not double-wrapped
17017+
assert!(!sql.contains("DATETIME(TIMESTAMP("));
17018+
17019+
let physical_plan = query_plan.as_physical_plan().await.unwrap();
17020+
println!(
17021+
"Physical plan: {}",
17022+
displayable(physical_plan.as_ref()).indent()
17023+
);
17024+
}
1697317025
}

0 commit comments

Comments
 (0)