Fix #892: materialize viral-propagation operand and unpivot window once#905
Open
albertohernandez1995 wants to merge 3 commits into
Open
Fix #892: materialize viral-propagation operand and unpivot window once#905albertohernandez1995 wants to merge 3 commits into
albertohernandez1995 wants to merge 3 commits into
Conversation
albertohernandez1995
enabled auto-merge (squash)
July 14, 2026 11:25
javihern98
reviewed
Jul 14, 2026
javihern98
left a comment
javihern98
left a comment
Contributor
There was a problem hiding this comment.
I am not really sure this actually gives any performance gain given the duckdb internal optimiser. I believe we should add as well some kind of comparison between the before and after using the reproducible example of the issue, as well as looking into the actual duckdb execution plan
Benchmarks (DS_1 1.2M / DS_2 3M rows, DuckDB threads=4) show the _op MATERIALIZED CTE regresses hierarchy ~12% by blocking filter pushdown, with no measurable win — DuckDB already plans the repeated inline operand efficiently. Revert that part; keep the unpivot _uv_src window dedup (EXPLAIN: WINDOW/SEQ_SCAN 3->1, scales with measure count).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Viral-propagation SQL repeated expensive work:
hierarchyre-embedded aderived operand (e.g. an inline
filter) once per leaf child, andunpivotrecomputed the dataset-wide viral window (
AGG(...) OVER ()) once per measurearm. Both now route through a single materialized CTE —
_opfor the hierarchyoperand (referenced by the pivot chain and the viral CTEs) and
_uv_srcfor theunpivot window (referenced by every measure arm) — so the operand subquery and
the window pass are evaluated once. The hierarchy change is gated on viral
attributes being present, so the measure-only path is byte-for-byte unchanged.
Checklist
ruff format,ruff check,mypy)pytest) — 5051 passed, 11 skippedFixes #892