Skip to content

fix(cubesql): Support DATE_ADD SQL pushdown #11539

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
MazterQyou wants to merge 1 commit into
base: master
Choose a base branch
from
+168 −1
Open

fix(cubesql): Support DATE_ADD SQL pushdown #11539

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/cubejs-databricks-jdbc-driver/src/DatabricksQuery.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,10 @@ export class DatabricksQuery extends BaseQuery {
templates.functions.LTRIM = 'LTRIM({{ args|reverse|join(", ") }})';
templates.functions.RTRIM = 'RTRIM({{ args|reverse|join(", ") }})';
templates.functions.DATEDIFF = 'DATEDIFF({{ date_part }}, DATE_TRUNC(\'{{ date_part }}\', {{ args[1] }}), DATE_TRUNC(\'{{ date_part }}\', {{ args[2] }}))';
// DATEADD is being rewritten to DATE_ADD. The unquoted multi-unit form is used
// because the ANSI form, INTERVAL '2' HOUR, only spans YEAR to SECOND, while sub-day
// intervals are reported in milliseconds
templates.functions.DATE_ADD = '({{ args[0] }} + INTERVAL {{ interval }} {{ date_part }})';
templates.functions.LEAST = 'LEAST({{ args_concat }})';
templates.functions.GREATEST = 'GREATEST({{ args_concat }})';
templates.functions.TRUNC = 'CASE WHEN ({{ args[0] }}) >= 0 THEN FLOOR({{ args_concat }}) ELSE CEIL({{ args_concat }}) END';
Expand Down
2 changes: 2 additions & 0 deletions packages/cubejs-duckdb-driver/src/DuckDBQuery.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ export class DuckDBQuery extends BaseQuery {
templates.functions.LEAST = 'LEAST({{ args_concat }})';
templates.functions.GREATEST = 'GREATEST({{ args_concat }})';
templates.functions.STRING_AGG = 'STRING_AGG({% if distinct %}DISTINCT {% endif %}{{ args[0] }}, COALESCE({{ args[1] }}, \'\'))';
// DATEADD is being rewritten to DATE_ADD
templates.functions.DATE_ADD = '({{ args[0] }} + \'{{ interval }} {{ date_part }}\'::interval)';
delete templates.functions.WIDTH_BUCKET;
templates.expressions.like = '{{ expr }} {% if negated %}NOT {% endif %}LIKE {{ pattern }}{% if default_escape %} ESCAPE \'\\\'{% endif %}';
templates.expressions.ilike = '{{ expr }} {% if negated %}NOT {% endif %}ILIKE {{ pattern }}{% if default_escape %} ESCAPE \'\\\'{% endif %}';
Expand Down
3 changes: 3 additions & 0 deletions packages/cubejs-schema-compiler/src/adapter/ClickHouseQuery.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,9 @@ export class ClickHouseQuery extends BaseQuery {
templates.functions.DATETRUNC = 'DATE_TRUNC({{ args_concat }})';
templates.functions.UTCTIMESTAMP = 'now(\'UTC\')';
templates.functions.STRING_AGG = 'arrayStringConcat(group{% if distinct %}Uniq{% endif %}Array({{ args[0] }}), {{ args[1] }})';
// DATEADD is being rewritten to DATE_ADD. The operator form is used instead of
// addDate(), which only exists since ClickHouse 23.9
templates.functions.DATE_ADD = '({{ args[0] }} + INTERVAL {{ interval }} {{ date_part }})';
// TODO: Introduce additional filter in jinja? or parseDateTimeBestEffort?
// https://github.com/ClickHouse/ClickHouse/issues/19351
templates.expressions.timestamp_literal = 'parseDateTimeBestEffort(\'{{ value }}\')';
Expand Down
2 changes: 2 additions & 0 deletions packages/cubejs-schema-compiler/src/adapter/MssqlQuery.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,8 @@ export class MssqlQuery extends BaseQuery {
templates.functions.UTCTIMESTAMP = 'GETUTCDATE()';
// MSSQL ROUND requires 2 arguments: ROUND(number, length)
templates.functions.ROUND = 'ROUND({{ args_concat }}{% if args | length < 2 %}, 0{% endif %})';
// DATEADD is being rewritten to DATE_ADD
templates.functions.DATE_ADD = 'DATEADD({{ date_part }}, {{ interval }}, {{ args[0] }})';
// NOTE: MSSQL does not support DISTINCT clause. No workaround is available
delete templates.functions.STRING_AGG;
// PERCENTILE_CONT works but requires PARTITION BY
Expand Down
5 changes: 5 additions & 0 deletions packages/cubejs-schema-compiler/src/adapter/MysqlQuery.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,11 @@ export class MysqlQuery extends BaseQuery {
const templates = super.sqlTemplates();
templates.functions.STRING_AGG = 'GROUP_CONCAT({% if distinct %}DISTINCT {% endif %}{{ args[0] }} SEPARATOR {{ args[1] }})';
templates.functions.UTCTIMESTAMP = 'UTC_TIMESTAMP()';
// DATEADD is being rewritten to DATE_ADD, which reports sub-day intervals in
// milliseconds. MySQL has no MILLISECOND unit, so those are scaled to microseconds
templates.functions.DATE_ADD = 'DATE_ADD({{ args[0] }}, INTERVAL '
+ '{% if date_part == "MILLISECOND" %}{{ interval }}000 MICROSECOND'
+ '{% else %}{{ interval }} {{ date_part }}{% endif %})';
// PERCENTILE_CONT works but requires PARTITION BY
delete templates.functions.PERCENTILECONT;
delete templates.functions.WIDTH_BUCKET;
Expand Down
2 changes: 1 addition & 1 deletion packages/cubejs-schema-compiler/src/adapter/PostgresQuery.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ export class PostgresQuery extends BaseQuery {
templates.functions.NOW = 'NOW({{ args_concat }})';
templates.functions.UTCTIMESTAMP = '(NOW() AT TIME ZONE \'UTC\')';
// DATEADD is being rewritten to DATE_ADD
// templates.functions.DATEADD = '({{ args[2] }} + \'{{ interval }} {{ date_part }}\'::interval)';
templates.functions.DATE_ADD = '({{ args[0] }} + \'{{ interval }} {{ date_part }}\'::interval)';

@claude claude Bot Aug 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NANOSECOND is a unit the generator can emit (the IntervalMonthDayNano branch in wrapper.rs:3168-3181), and Postgres has no nanosecond interval unit — '7200000000000 NANOSECOND'::interval errors out. Same for DuckDB (microsecond granularity) and Trino.

Worth checking whether that branch is reachable for the interval literals DataFusion actually produces here; if it is, converting nanos to microseconds/seconds on the Rust side (before it reaches any dialect template) would fix all nine dialects at once rather than each template guarding independently. Note this also inherits into CrateQuery/FireboltQuery/QuestQuery, which extend PostgresQuery.

// TODO: is DATEDIFF expr worth documenting?
templates.functions.DATEDIFF = 'CASE WHEN LOWER(\'{{ date_part }}\') IN (\'year\', \'quarter\', \'month\') THEN (EXTRACT(YEAR FROM AGE(DATE_TRUNC(\'{{ date_part }}\', {{ args[2] }}), DATE_TRUNC(\'{{ date_part }}\', {{ args[1] }}))) * 12 + EXTRACT(MONTH FROM AGE(DATE_TRUNC(\'{{ date_part }}\', {{ args[2] }}), DATE_TRUNC(\'{{ date_part }}\', {{ args[1] }})))) / CASE LOWER(\'{{ date_part }}\') WHEN \'year\' THEN 12 WHEN \'quarter\' THEN 3 WHEN \'month\' THEN 1 END ELSE EXTRACT(EPOCH FROM DATE_TRUNC(\'{{ date_part }}\', {{ args[2] }}) - DATE_TRUNC(\'{{ date_part }}\', {{ args[1] }})) / EXTRACT(EPOCH FROM \'1 {{ date_part }}\'::interval) END::bigint';
templates.expressions.interval = 'INTERVAL \'{{ interval }}\'';
Expand Down
2 changes: 2 additions & 0 deletions packages/cubejs-schema-compiler/src/adapter/PrestodbQuery.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,8 @@ export class PrestodbQuery extends BaseQuery {
templates.functions.DATETRUNC = 'DATE_TRUNC({{ args_concat }})';
templates.functions.DATEPART = 'DATE_PART({{ args_concat }})';
templates.functions.DATEDIFF = 'DATE_DIFF(\'{{ date_part }}\', {{ args[1] }}, {{ args[2] }})';
// DATEADD is being rewritten to DATE_ADD
templates.functions.DATE_ADD = 'DATE_ADD(\'{{ date_part }}\', {{ interval }}, {{ args[0] }})';
templates.functions.CURRENTDATE = 'CURRENT_DATE';
templates.functions.UTCTIMESTAMP = 'CAST(NOW() AT TIME ZONE \'UTC\' AS TIMESTAMP)';
templates.functions.TRUNC = 'TRUNCATE({{ args_concat }})';
Expand Down
2 changes: 2 additions & 0 deletions packages/cubejs-schema-compiler/src/adapter/RedshiftQuery.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ export class RedshiftQuery extends PostgresQuery {
// nodes, unlike NOW(), which is a leader node–only function.
templates.functions.UTCTIMESTAMP = 'GETDATE()';
templates.functions.DATEDIFF = 'DATEDIFF({{ date_part }}, {{ args[1] }}, {{ args[2] }})';
// DATEADD is being rewritten to DATE_ADD
templates.functions.DATE_ADD = 'DATEADD({{ date_part }}, {{ interval }}, {{ args[0] }})';
templates.functions.STRING_AGG = 'LISTAGG({% if distinct %}DISTINCT {% endif %}{{ args_concat }})';
templates.statements.time_series_select = 'SELECT dates.f::timestamp date_from, dates.t::timestamp date_to \n' +
'FROM (\n' +
Expand Down
2 changes: 2 additions & 0 deletions packages/cubejs-schema-compiler/src/adapter/SnowflakeQuery.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@ export class SnowflakeQuery extends BaseQuery {
templates.functions.CHARACTERLENGTH = 'LENGTH({{ args[0] }})';
templates.functions.BTRIM = 'TRIM({{ args_concat }})';
templates.functions.STRING_AGG = 'LISTAGG({% if distinct %}DISTINCT {% endif %}{{ args_concat }})';
// DATEADD is being rewritten to DATE_ADD
templates.functions.DATE_ADD = 'DATEADD({{ date_part }}, {{ interval }}, {{ args[0] }})';
templates.expressions.extract = 'EXTRACT({{ date_part }} FROM {{ expr }})';
// Snowflake `/` is decimal division even for integer operands (output scale
// is dividend scale + 6), while this template must keep PostgreSQL integer
Expand Down
145 changes: 145 additions & 0 deletions rust/cubesql/cubesql/src/compile/test/test_wrapper.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2853,3 +2853,148 @@ async fn test_wrapper_only_system_fields() {
displayable(physical_plan.as_ref()).indent()
);
}

/// A per-group aggregate in a CTE, date-filtered and counted on the outside, must be
/// pushed down whole: `DATEADD` in the outer filter is rewritten to `DATE_ADD`, so the
/// filter and the aggregate above it only push down when the data source has a
/// `functions/DATE_ADD` template.
#[tokio::test]
async fn test_wrapper_cte_aggregate_then_date_filter() {
if !Rewriter::sql_push_down_enabled() {
return;
}
init_testing_logger();

let query_plan = convert_select_to_query_plan(
r#"
WITH first_orders AS (
SELECT customer_gender, MIN(order_date) AS first_order_at
FROM KibanaSampleDataEcommerce
WHERE has_subscription = true
GROUP BY 1
)
SELECT COUNT(DISTINCT customer_gender) AS customers
FROM first_orders
WHERE first_order_at >= DATEADD('month', -12, CURRENT_DATE())
AND first_order_at < CURRENT_DATE()
"#
.to_string(),
DatabaseProtocol::PostgreSQL,
)
.await;

let logical_plan = query_plan.as_logical_plan();
let sql = logical_plan.find_cube_scan_wrapped_sql().wrapped_sql.sql;
assert!(
sql.contains("COUNT(DISTINCT"),
"outer aggregate is pushed down: {}",
sql
);
assert!(
sql.contains("DATE_ADD"),
"outer date filter is pushed down: {}",
sql
);
Comment thread
claude[bot] marked this conversation as resolved.

let _physical_plan = query_plan.as_physical_plan().await.unwrap();
}

/// `DATEADD` is rewritten to `date_add`, and the dialect template renders it from the
/// `date_part` and `interval` variables rather than from the arguments. The rewrite maps
/// every unit onto one of three parts - sub-day units become `MILLISECOND`, `day` and
/// `week` become `DAY`, and `month`, `quarter` and `year` become `MONTH` - so each dialect
/// has to render all three. These are the real templates from the query classes; the ones
/// used elsewhere in these tests take `args_concat` and would not catch a wrong unit.
#[tokio::test]
async fn test_wrapper_date_add_dialect_templates() {
if !Rewriter::sql_push_down_enabled() {
return;
}
init_testing_logger();

let dialects = [
(
// PostgresQuery, RedshiftQuery inherits it, DuckDBQuery repeats it
"({{ args[0] }} + '{{ interval }} {{ date_part }}'::interval)",
[
"(CURRENT_DATE() + '7200000 MILLISECOND'::interval)",
"(CURRENT_DATE() + '14 DAY'::interval)",
"(CURRENT_DATE() + '24 MONTH'::interval)",
],
),
(
// SnowflakeQuery, MssqlQuery, RedshiftQuery
"DATEADD({{ date_part }}, {{ interval }}, {{ args[0] }})",
[
"DATEADD(MILLISECOND, 7200000, CURRENT_DATE())",
"DATEADD(DAY, 14, CURRENT_DATE())",
"DATEADD(MONTH, 24, CURRENT_DATE())",
],
),
(
// MysqlQuery: MySQL has no MILLISECOND unit, so those become microseconds
"DATE_ADD({{ args[0] }}, INTERVAL {% if date_part == \"MILLISECOND\" %}\
{{ interval }}000 MICROSECOND{% else %}{{ interval }} {{ date_part }}{% endif %})",
[
"DATE_ADD(CURRENT_DATE(), INTERVAL 7200000000 MICROSECOND)",
"DATE_ADD(CURRENT_DATE(), INTERVAL 14 DAY)",
"DATE_ADD(CURRENT_DATE(), INTERVAL 24 MONTH)",
],
),
(
// ClickHouseQuery, DatabricksQuery
"({{ args[0] }} + INTERVAL {{ interval }} {{ date_part }})",
[
"(CURRENT_DATE() + INTERVAL 7200000 MILLISECOND)",
"(CURRENT_DATE() + INTERVAL 14 DAY)",
"(CURRENT_DATE() + INTERVAL 24 MONTH)",
],
),
(
// PrestodbQuery, TrinoQuery and AthenaQuery inherit it
"DATE_ADD('{{ date_part }}', {{ interval }}, {{ args[0] }})",
[
"DATE_ADD('MILLISECOND', 7200000, CURRENT_DATE())",
"DATE_ADD('DAY', 14, CURRENT_DATE())",
"DATE_ADD('MONTH', 24, CURRENT_DATE())",
],
),
];

for (template, expected) in dialects {
for (unit, expected) in ["hour", "week", "year"].iter().zip(expected) {

@claude claude Bot Aug 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This matrix is a big improvement over the args_concat fixture — it genuinely pins arg order and both context variables per dialect. One gap: every case uses a positive interval (2), which is exactly where the sub-day encoding happens to work.

DATEADD('hour', -2, ...) goes through redshift-dateadd-to-interval → transform_interval_parts_to_interval (rust/cubesql/cubesql/src/compile/rewrite/rules/dates.rs:539-541), which builds the literal as IntervalDayTime(3_600_000 * n). IntervalDayTime is a packed (days: i32, millis: i32) pair, not a scalar, so a plain multiply only round-trips for non-negative values that fit in 31 bits:

  • n = -2 → -7_200_000i64 = 0xFFFF_FFFF_FF91_7A00 → days = -1, ms = -7_200_000. wrapper.rs:3151 then hits the mixed branch and returns Unsupported mixed IntervalDayTime: days = -1, ms = -7200000.
  • n = 700 (hours) → 2_520_000_000 > i32::MAX → ms as i32 wraps to -1_774_967_296, so the pushed-down SQL silently gets a negative interval of the wrong magnitude.

Neither is introduced by this PR — the same literal is what DataFusion evaluates in the post-processing path today, so the value is equally wrong without pushdown — but this test is the natural place to catch it, and negative offsets (DATEADD('hour', -1, ...), DATEADD('minute', -30, ...)) are the common shape in real queries. Adding -2 alongside 2 to the unit loop would fail today.

The underlying fix is a one-liner at the construction site — build the two halves separately instead of multiplying:

Some("hour") => {
    let ms = i32::try_from(3_600_000i64 * i64::from(interval_int)).ok()?; // bail if it overflows
    ScalarValue::IntervalDayTime(Some(i64::from(ms) & 0xFFFF_FFFF))
}

(and the same shape for millisecond / second / minute; day / week are exact multiples of 2^32 so they already sign-extend correctly). Happy to be wrong here if there's a normalization step I've missed — but if it holds, it's worth either fixing in this PR or filing separately, since this PR is what makes the value visible in generated SQL.

// A filter over a per-group aggregate cannot become a Cube filter, so the
// whole expression has to be rendered by the template
let sql = convert_select_to_query_plan_customized(
format!(
r#"
WITH first_orders AS (
SELECT customer_gender, MIN(order_date) AS first_order_at
FROM KibanaSampleDataEcommerce
GROUP BY 1
)
SELECT COUNT(DISTINCT customer_gender) AS customers
FROM first_orders
WHERE first_order_at > DATEADD('{unit}', 2, CURRENT_DATE())
"#
),
DatabaseProtocol::PostgreSQL,
vec![("functions/DATE_ADD".to_string(), template.to_string())],
)
.await
.as_logical_plan()
.find_cube_scan_wrapped_sql()
.wrapped_sql
.sql;

assert!(
sql.contains(expected),
"`{}` renders as `{}` with template `{}`, got: {}",
unit,
expected,
template,
sql
);
}
}
}
Loading