Repository navigation
Keep the DuckDB IEJoin plan for scalar expression projections instead of declining to the naive plan — Closes #109 - #235
Merged
Conversation
This was referenced Sep 3, 2026
A column-to-column INTERSECTS join whose SELECT list held a scalar expression over qualified columns declined to the naive predicate, even though the join itself was an INNER pure-inequality join the dialect already routes through IE_JOIN. The projection shape alone cost the query its plan: a hash join on chrom with the position inequalities as a residual filter, quadratic when the chromosome key has low cardinality. That gate answers whether the projection rebuild can express a shape, and it was also deciding which join plan the query gets, though the projection takes no part in join planning. The rebuild needed nothing new. A plain aggregate already projects its argument columns inward as __giql_p<n> inner columns and re-emits the call over them on the outer wrapper SELECT; the same two steps applied to the leaves of an expression tree serve GREATEST, LEAST, arithmetic, CASE, casts and concatenation alike, because the wrapper relation carries exactly the leaf values the per-chromosome join produced. An unaliased expression is named __giql_expr_<n>, as an unaliased aggregate is named __giql_agg_<n>, since DuckDB's automatic name is derived from text the rewrite has changed. This costs two of the twelve bedtools intersect modes in the benchmark suite: the default clipped overlap span and -wo, both quadratic today. The decline from #205 is narrowed rather than reverted. A window aggregate, a FILTER clause, a scalar subquery, an aggregate nested in an expression, a bare literal and a star anywhere in the tree still fall back to the naive plan, so dialect="duckdb" stays consistent with every other backend. An out-of-scope leaf still raises: unqualified, unknown table, or a right-side reference under SEMI or ANTI, which now reaches the same diagnostic from anywhere in the tree rather than only at its root. The outer-join decomposition stays column-only, since its unmatched half would have to evaluate the expression over NULL-filled leaves from the other side.
The four tests that pinned an expression projection as a decline now assert it engages, and a.start + 1 leaves the unsupported-projection parametrizations. What replaces it there is the boundary the rebuild actually draws: each carve-out nested one level inside an otherwise rebuildable tree, since a window aggregate or a subquery reached through a GREATEST must decline as surely as one at the root; literal-only trees, which have no leaf to attribute to a side; and an expression sharing a SELECT list with a star or a window aggregate, where the older declines still win. The deep-leaf cases matter for the same reason on the error side: an unqualified or right-side column must raise from anywhere in the tree, not only where it used to sit. The execution tests cover what the property strategy cannot reach, because it generates integer arithmetic only: string concatenation, a boolean comparison, a CASE over literals and a cast. Alongside them the compositions the rebuild has to survive, each verified against the naive plan in rows and result schema: ANTI with a WHERE residual layered on the wrapper, an implicit comma join, DISTINCT over a rebuilt value, GROUP BY on an expression rather than on its leaf, LIMIT and OFFSET behind an ORDER BY alias, USING, custom column names, one-based closed coordinates where the rebuilt expression sees raw values while the join sees canonical ones, and case-folded user aliases. Four invariants are pinned that no single-shape test reaches. Both left-only shapes still plan through IE_JOIN rather than the nested loop the correlated EXISTS form exists to avoid. The unpartitioned branch above the contig ceiling carries the same projection list as the per-chromosome one. The empty-schema fallback types an expression column from the source tables under WHERE FALSE. And two queries whose inner projections coincide share one content-addressed session variable without contaminating each other's results, which is what makes that sharing safe now that the outer SELECT can differ while the inner one does not. Two property tests join the existing one. The first draws expression trees over left-only leaves under SEMI and ANTI with a WHERE residual and an ORDER BY, the path where a right-side leaf must raise instead. The second checks the clipped-span and overlap-width shapes against a Python reference rather than against the naive plan, so the bedtools semantics are pinned by something other than the implementation they are compared with.
These are the two modes the expression rebuild exists to serve, and nothing in the lane exercised them: the intersect oracle covered the join shapes whose projections are plain columns, which is every shape that already reached the fast path. Both new cases assert the dialect engaged as well as that the rows agree, because either would keep passing on the naive predicate and so would report nothing about the rewrite. The wrapper gains a clip mode for the first of them. Every existing caller selects a bedtools flag, and the default output with no flag at all -- each A feature clipped to its overlap -- had no way to be requested; the branch passes no flag and reads the result back through the same bed6 path the other flagless modes use.
The guide listed an expression among the shapes that fall back, which is now the opposite of what the dialect does, and a caller reading it would spell out a clipped overlap span expecting the naive predicate and get the fast path instead. Expression projections join the decorations the dialect absorbs, naming the bedtools modes they serve and the synthesized output name an unaliased one carries. The fallback entry keeps the shapes that still decline and now says so as a closed list rather than an open one, since the distinction between a rebuildable expression and a wrapper the rebuild cannot express is the whole of what a caller needs to predict the plan. Two boundaries are stated where they would otherwise surprise: an expression under a LEFT or RIGHT join still falls back, because the decomposition's unmatched half only NULL-fills columns; and an unqualified column inside an expression still raises, where the guide previously said every such wrapper falls back once qualified.
conradbzura
force-pushed
the
109-iejoin-scalar-expr-projections
branch
from
September 3, 2026 16:33
8b58119 to
23ca2f8
Compare
The LEFT-join scale test divides two wall-clock measurements and asserts the ratio stays under 2.5x. That bound has to sit between the 2.0x the decomposition should achieve and the 3.3x regression it exists to catch (#95), so the window it discriminates is narrow -- narrower than a shared runner's variance on a single sample. CI produced 2.64x for an implementation that measures 1.6x unloaded, failing the assertion without any change to the SQL either plan emits. The two plans also do not degrade alike under load. The dialect plan is a multi-statement per-chromosome UNION ALL with far more parallelism to lose than the naive plan's single hash join, so contention inflates the ratio by construction rather than symmetrically. Widening the bound is therefore not an option: it would have to cross into the band the regression occupies. Take the minimum of several runs per plan instead. Contention inflates a sample upward and never downward, so the minimum estimates the uncontended cost and the ratio stops depending on whether a stall happened to land in one measurement and not the other. The 2.5x bound is unchanged and still catches the regression. Costs about 14s on the two tests that measure ratios; the structural INTERSECT / NOT IN assertions that pin the mechanism are untouched.
conradbzura
marked this pull request as ready for review
September 3, 2026 19:44
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
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
Under
dialect="duckdb", a column-to-columnINTERSECTSjoin whose SELECT list carried a scalar expression over qualified columns declined to the naive-predicate plan, even though the join itself was IEJoin-able. Keep the IEJoin plan for these: project each leaf column into the per-chromosome subquery as an existing__giql_p<n>inner column, and rebuild the expression tree on the outer wrapperSELECTover those. The join is untouched. This puts the twobedtools intersectshapes the issue names — the default clipped overlap span, and-wopairs plus overlap width — on the fast path.Acceptance is a property rather than a list of blessed shapes. A projection is rebuilt when it is row-wise, binds no names, carries no star, has at least one qualified column leaf, and DuckDB's generator reproduces it faithfully. The first four are node-class checks in
_NON_REBUILDABLE_NODES, grouped so each entry states the property it violates. The fifth cannot be predicted from a class list — it is a property of the parse/generate dialect pair — so it is verified: re-emit the subtree with DuckDB's generator, re-read it withGIQLDialect, and decline unless the tree comes back unchanged.Two trade-offs worth stating. The leaves rather than the computed value go inward deliberately: the session-variable name is a content digest of the inner SQL, so computing the expression inside the subquery would fold its text into the digest and mint a distinct
SET VARIABLEper expression on a long-lived connection, trading a bounded variable set for an unbounded one. And rejecting functions sqlglot does not model costs the fast path for exotic calls, but it is what keeps the published__giql_agg_<n>naming contract true — 29 of DuckDB's 88 aggregates parse as anonymous calls and would otherwise be rebuilt as expressions.The #205 decline is narrowed, not reverted. Window aggregates,
FILTERclauses, scalar subqueries, aggregates nested in expressions and aggregate-nested stars all still decline, and an out-of-scope column reference still raises.Closes #109.
Proposed changes
Rebuild scalar expressions on the outer wrapper SELECT. Pre-allocate every leaf of a non-column SELECT item as a
__giql_p<n>inner column, then re-emit the tree against those aliases. An aliased expression keeps its alias; an unaliased one is named__giql_expr_<n>, mirroring__giql_agg_<n>, because DuckDB's automatic name would be derived from rewritten text naming inner columns the caller never wrote.Decide each SELECT item once.
_classify_projectionreturns a frozen_ProjectionPlancarrying one verdict —column,aggregate,scalar,declineorraise— and the dispatch switches on it. Previously the question "are this tree's leaves attributable to a join side?" was answered by three independently written formulations, and the two that ran later were correct only because the first had already run.Gate acceptance on a verified property.
_is_rebuildable_scalarchecks the reject tuple, the qualified-leaf requirement, and_renders_faithfully_for_duckdb. The reject tuple's groups are: not row-wise (AggFunc,Window,Unnest,Explode); aggregates sqlglot models outsideAggFunc(List,RowNumber); binds names (Select,Subquery,Lambda); not enumerable (Star); unrecognized (Anonymous).Name synthesized columns without shadowing the caller's.
__giql_expr_<n>and__giql_agg_<n>are chosen around the output names the user actually wrote, viasqlglot.helper.find_new_name— the same collision resolvercanonicalizer.pyalready uses. Two same-named user columns (SELECT a.start, b.start) are left alone, since the naive plan reports that duplicate too and the dialect must not be stricter than the plan it accelerates.One diagnosis site for predicates and projections. Extract
_raise_unattributable_columnfrom_validate_extra_qualifiersand call it from both, so a projection error names the offending leaf —GREATEST(a.start, c.start)reportsc.startrather than prescribing that an already-qualified column be qualified. A catalog-qualified projection leaf now raises the same error a catalog-qualified predicate leaf already did, restoring the rule that the dialect is never more permissive than the naive plan.Deduplicate inner columns case-insensitively.
allocatefolds both halves of its key, soGREATEST(a.START, a.start)allocates one__giql_p<n>instead of carrying a duplicate of one value per row through every branch of the per-chromosome union.Documentation. Restate the
performance.rstcontract as the property the code checks, with the fall-back list reorganized by which property each shape fails. Note intranspile()that this dialect synthesizes names for unaliased SELECT items and that aliasing is how a caller pins them.Test-support consolidation. Collapse 11 inlined copies of
_make_interval_tables, three copies of the planner-scale table setup into_make_planner_scale_tables, and seven inlined interval-list strategies into_interval_lists. Compose_python_clipped_overlapon_python_overlapso the half-open predicate is defined once. Hoist the shared bedtools interval pair into apartial_overlap_intervalsfixture.Test cases
424 tests collected across the two files, against 293 on
main.SQLStructuredialect='duckdb'SQLStructuredialect='duckdb'SQLStructuredialect='duckdb'__giql_expr_<n>by positionSQLStructuredialect='duckdb'SQLStructuredialect='duckdb'__giql_p<n>SQLStructuredialect='duckdb'SQLStructuredialect='duckdb'SQLStructuredialect='duckdb'ProjectionContractdialect='duckdb'ProjectionContractdialect='duckdb'ProjectionContractduckdb_functions()dialect='duckdb'ProjectionContractdialect='duckdb'ProjectionContractdialect='duckdb'ProjectionContractdialect='duckdb'ProjectionContractdialect='duckdb'ProjectionContractCASEor a calldialect='duckdb'ExecutionExecutionUSINGExecutionDISTINCT,GROUP BY,HAVING,ORDER BY,LIMITExecutionExecutionExecutionExecutionExecutionIE_JOIN, notBLOCKWISE_NL_JOINExecutionExecutiondialect='duckdb'executedOuterJoinDecompositiondialect='duckdb'-waoshape runs correctlyOuterJoinDecompositiontransform_to_partsis calledCountOverlapscount_overlapskeydialect='duckdb'SET VARIABLEUnsupportedProjectionFallbackFILTER, subquery, nested-aggregate, star, subscript, lambda and anonymous-aggregate shapesdialect='duckdb'bedtools(integration)-woprojections executedbedtoolsexactlyRelated
Reviewing this PR surfaced a defect outside its scope: every literal subscript shifts by one under
dialect="duckdb", becauseGIQLDialectinherits sqlglot's 0-basedINDEX_OFFSETwhile DuckDB's generator is 1-based. It affects any query, with no join or operator involved, and predates this branch. Filed as #239 with a validated two-line fix. This PR does not fix it, but the round-trip gate above prevents the projection rebuild from becoming a second source of it, and pins that with a decline test.Follow-ups also filed from the round-1 review: #236 (bedtools test-style conversion), #237 (engage/decline observability), #238 (bedtools flag-chain refactor).