diff --git a/README.md b/README.md index 66be572..75c3136 100644 --- a/README.md +++ b/README.md @@ -71,7 +71,7 @@ print(sql) The transpiled SQL can be executed with fast genome-unaware databases or in-memory analytic engines like DuckDB. -By default a column-to-column `INTERSECTS` join emits the naive overlap predicate (`a.chrom = b.chrom AND a.start < b.end AND b.start < a.end`) as a plain `ON` condition, which each engine's optimizer plans as a range join. For DuckDB you can additionally pass `dialect="duckdb"` to opt into a per-chromosome IEJoin plan for INNER, SEMI, or ANTI joins; shapes it declines fall through to the naive predicate. See [DuckDB IEJoin Dialect](https://giql.readthedocs.io/en/latest/transpilation/performance.html#duckdb-iejoin-dialect) for the supported shapes and fallback rules. +By default a column-to-column `INTERSECTS` join emits the naive overlap predicate (`a.chrom = b.chrom AND a.start < b.end AND b.start < a.end`) as a plain `ON` condition, which each engine's optimizer plans as a range join. For DuckDB you can additionally pass `dialect="duckdb"` to opt into a per-chromosome IEJoin plan for INNER, SEMI, ANTI, and LEFT/RIGHT outer joins; shapes it declines fall through to the naive predicate. See [DuckDB IEJoin Dialect](https://giql.readthedocs.io/en/latest/transpilation/performance.html#duckdb-iejoin-dialect) for the supported shapes and fallback rules. You can also use [oxbow](https://oxbow.readthedocs.io) to efficiently stream specialized genomics formats into DuckDB. diff --git a/docs/dialect/spatial-operators.rst b/docs/dialect/spatial-operators.rst index 3e0a107..c670639 100644 --- a/docs/dialect/spatial-operators.rst +++ b/docs/dialect/spatial-operators.rst @@ -104,7 +104,7 @@ Join Internals A column-to-column ``INTERSECTS`` join expands to the **naive overlap predicate** — ``a.chrom = b.chrom AND a.start < b.end AND b.start < a.end`` — left in place as an ordinary ``ON`` (or ``WHERE``) condition. GIQL emits standard SQL and lets each engine's optimizer plan the range join: on DuckDB and DataFusion this becomes a hash join keyed on ``chrom`` with the two position inequalities applied as a residual join filter. Because the predicate is a plain condition on the original tables, inner and outer joins (``LEFT`` / ``RIGHT`` / ``FULL``) get correct semantics — including unmatched-row preservation — with no special restructuring, and every output row's multiplicity follows standard SQL bag semantics with no ``DISTINCT`` workaround. -DuckDB additionally offers an opt-in ``dialect="duckdb"`` per-chromosome IEJoin plan for INNER / SEMI / ANTI joins that routes through its ``IE_JOIN`` / ``PIECEWISE_MERGE_JOIN`` family; shapes it declines fall through to the naive predicate. See the :doc:`performance guide ` for details. +DuckDB additionally offers an opt-in ``dialect="duckdb"`` per-chromosome IEJoin plan for INNER / SEMI / ANTI joins, and for LEFT / RIGHT outer joins via a two-half decomposition, that routes through its ``IE_JOIN`` / ``PIECEWISE_MERGE_JOIN`` family; shapes it declines (including ``FULL OUTER``) fall through to the naive predicate. See the :doc:`performance guide ` for details. Related Operators ~~~~~~~~~~~~~~~~~ diff --git a/docs/recipes/bedtools-migration.rst b/docs/recipes/bedtools-migration.rst index 9c07d72..f27a4cf 100644 --- a/docs/recipes/bedtools-migration.rst +++ b/docs/recipes/bedtools-migration.rst @@ -268,6 +268,15 @@ Default: Report overlaps between A and B FROM features_a a LEFT JOIN features_b b ON a.interval INTERSECTS b.interval +.. note:: + + Under ``dialect="duckdb"`` this star form takes the naive-predicate plan, + not the outer-join fast path: the decomposition needs every projection to + be attributable to one side, and GIQL does not read table schemas, so it + cannot enumerate ``a.*`` into columns. Spell the columns out + (``SELECT a.chrom, a.start, a.end, ..., b.chrom AS b_chrom, ...``) to get + the accelerated plan. See :doc:`../transpilation/performance`. + ``-s``: Same strand overlaps only ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/docs/transpilation/performance.rst b/docs/transpilation/performance.rst index 1fcd643..af319c5 100644 --- a/docs/transpilation/performance.rst +++ b/docs/transpilation/performance.rst @@ -313,12 +313,13 @@ as a residual join filter, correct for both inner and outer joins with no special handling. This is the lowest-common-denominator plan: standard SQL that every target runs. -For column-to-column ``INTERSECTS`` joins (INNER, SEMI, or ANTI) on -DuckDB, the ``dialect="duckdb"`` opt-in instead transpiles the join into a -per-chromosome dynamic-SQL pattern. Each per-chromosome subquery contains -only inequality predicates, which DuckDB plans through its range-join -family (``IE_JOIN`` or ``PIECEWISE_MERGE_JOIN``). Shapes this path declines -fall through to the naive predicate above. +For column-to-column ``INTERSECTS`` joins (INNER, SEMI, ANTI, and LEFT / +RIGHT outer) on DuckDB, the ``dialect="duckdb"`` opt-in instead +transpiles the join into a per-chromosome dynamic-SQL pattern. Each +per-chromosome subquery contains only inequality predicates, which DuckDB +plans through its range-join family (``IE_JOIN`` or +``PIECEWISE_MERGE_JOIN``). Shapes this path declines fall through to the +naive predicate above. .. code-block:: python @@ -333,17 +334,21 @@ fall through to the naive predicate above. tables=["peaks", "genes"], dialect="duckdb", ) - # sql is a two-statement script — see the next example for execution. + # sql is a multi-statement script — see the next example for execution. The output is a multi-statement string of the form:: SET VARIABLE __giql_iejoin_ = COALESCE((... string_agg per chromosome ...), ''); SELECT ... FROM query(getvariable('__giql_iejoin_')) AS __giql_iejoin_wrapper -The ```` is a per-call ``uuid4().hex`` (128 bits) suffix so the -``SET VARIABLE`` name is collision-resistant even when outputs from many -``transpile()`` calls are interleaved in a single DuckDB session -(session variables are global session state). The outer SELECT's +Each ```` is a 128-bit digest of the variable's own rendered value, +so the ``SET VARIABLE`` name stays collision-resistant when outputs from +many ``transpile()`` calls are interleaved in a single DuckDB session — an +outer-join decomposition emits two such variables, one per half. Because +the name addresses the content, re-running one query shape rebinds its own +names rather than leaving a fresh pair behind on every call, which matters +because session variables are global session state that the emitted script +cannot release: the final statement has to be the ``SELECT``. The outer SELECT's wrapper-relation alias is constant (``__giql_iejoin_wrapper``) because it isn't user-visible and doesn't need to vary per call. @@ -352,11 +357,13 @@ via ``replace(chrom, '''', '''''')`` wrapped in single quotes, so chromosome identifiers containing literal single quotes interpolate safely into the dynamic SQL. -Because the output is a two-statement script that depends on session -state, execute it through a single DuckDB connection's ``.execute()`` — -DuckDB ≥1.4 (the version pinned by GIQL's development and CI -environment) accepts multi-statement strings in one call and returns -the result of the final statement. SQLAlchemy's ``text()``, +The output is a multi-statement script that depends on session state: two +statements for the shapes above, and three for the outer-join +decomposition, which declares one session variable per half. Execute it +through a single DuckDB connection's ``.execute()``. DuckDB ≥1.4 (the +version pinned by GIQL's development and CI environment) accepts +multi-statement strings in one call and returns the result of the final +statement. SQLAlchemy's ``text()``, ``pandas.read_sql_query``, and similar wrappers that split or rewrite the string may drop the ``SET VARIABLE`` and produce empty or NULL results. @@ -382,11 +389,12 @@ results. The dialect rewrites the whole query, so the supported shape is ``SELECT FROM -{INNER|SEMI|ANTI} JOIN {ON|WHERE} `` — the -JOIN and the column-to-column ``INTERSECTS`` are both required -(literal-range ``INTERSECTS`` against a single table falls through to -the default predicate generator). +{INNER|SEMI|ANTI|LEFT|RIGHT} JOIN {ON|WHERE} `` — the JOIN and the column-to-column +``INTERSECTS`` are both required (literal-range ``INTERSECTS`` against a +single table falls through to the default predicate generator). ``LEFT`` +and ``RIGHT`` accept ``ON`` only: the outer-join decomposition rejects any +top-level ``WHERE``, for the reason given under **Outer joins** below. **Join variants.** ``INNER JOIN`` is the default. ``SEMI JOIN`` returns distinct left rows that have at least one overlapping match; ``ANTI @@ -394,10 +402,12 @@ JOIN`` returns left rows with no overlapping match. Both restrict the outer SELECT to left-side projections — any ``b.col`` reference (or an aggregate over ``b.col``) raises ``ValueError``; a right-side ``b.*`` declines to the naive-predicate plan with every other star (which then -rejects the out-of-scope right table at bind time). ANTI uses a -left-distinct chromosome partition rather than the chromosome INTERSECT -used by INNER / SEMI, so left rows on chromosomes absent from the right -table are preserved. +rejects the out-of-scope right table at bind time). ANTI partitions on the +same chromosome ``INTERSECT`` as INNER and SEMI, then unions one +non-partitioned branch carrying the left rows whose chromosome the right +table lacks. Those rows cannot match, so giving each such chromosome its +own branch would scan both tables to prove an emptiness the partition +already knows. DuckDB plans all three variants through its ``IE_JOIN`` / ``PIECEWISE_MERGE_JOIN`` sort-merge range-join family. INNER emits a @@ -408,24 +418,94 @@ JOIN`` / ``ANTI JOIN`` inequality overlap as a quadratic ``BLOCKWISE_NL_JOIN`` (a nested loop) rather than the range-join fast path — the correlated form reaches the fast path while preserving exact semantics (#208). Expect large speedups over the naive predicate for all -three variants at scale. +three variants at scale, subject to the contig-cardinality caveat below. + +**The contig-cardinality ceiling.** The rewrite emits one ``UNION ALL`` +branch per distinct chromosome, so its cost scales with that cardinality +while the plain predicate's does not. That is the whole trade: on a +primary assembly the partitioned form wins by orders of magnitude, and on +a scaffold-level one it loses by them. Measured on 262,144 intervals per +side, LEFT join, partitioned against naive: 0.79s against 4.53s at 24 +contigs, roughly level at 150, then 9.75s against 0.18s at 1,000 and +57.2s against 0.11s at 3,000. + +The partition's cardinality is a property of the data, which no +transpile-time check can see, so the choice is made at execution time. The +``SET VARIABLE`` statement counts the partition it is about to build and, +above ``MAX_PER_CHROM_PARTITIONS`` (256), binds the same query with the +chromosome equality inlined rather than partitioned out. The ceiling sits +above every standard reference assembly's primary-chromosome count and +below the measured crossover, so a normal genome takes the fast form and a +scaffolded one degrades to the plan it would have had anyway. **count_overlaps.** The idiomatic per-interval overlap count — ``SELECT a.cols, COUNT(b.col) FROM a LEFT JOIN b ON a.interval INTERSECTS b.interval GROUP BY a.cols`` — is also accelerated (#209). A plain -``LEFT JOIN`` inequality overlap would decline to the naive predicate (a -``HASH_JOIN`` on ``chrom`` with the position inequalities as a residual -filter, quadratic when the chromosome key has low cardinality). Instead -the dialect computes the counts through the INNER ``IE_JOIN`` path and +``LEFT JOIN`` inequality overlap would otherwise decline to the naive +predicate outright — a ``HASH_JOIN`` on ``chrom`` with the position +inequalities as a residual filter, quadratic when the chromosome key has +low cardinality. The outer-join decomposition below does not rescue it: +that rewrite rejects any ``GROUP BY``, so this shape never reaches it. +Instead the dialect computes the counts through the INNER ``IE_JOIN`` path +and wraps them in a zero-fill ``LEFT JOIN`` back onto the distinct left keys, so left intervals with no overlap report ``0``. This covers a single ``COUNT()`` (or ``COUNT(DISTINCT )``) with a -``GROUP BY`` on the projected left columns; ``COUNT(*)``, non-COUNT -aggregates, ``ORDER BY`` / ``LIMIT`` / ``HAVING``, and other outer-join -shapes still take the naive-predicate plan. - -On top of the core shape the dialect also absorbs several common -decorations: +``GROUP BY`` on the projected left columns. ``COUNT(*)``, non-COUNT +aggregates, and ``ORDER BY`` / ``LIMIT`` / ``HAVING`` fall out of this fast +path, and each of them is declined by the outer-join decomposition too, so +they take the naive-predicate plan. + +**Outer joins.** A ``LEFT`` / ``RIGHT JOIN ... ON a.interval INTERSECTS +b.interval`` whose projections are all side-attributable qualified columns +is decomposed rather than emitted as an outer join (#95): the matched pairs +come from the INNER ``IE_JOIN`` path, and the preserved side's unmatched +rows come from the ``NOT EXISTS`` ANTI path, combined with ``UNION ALL`` +and NULL-filled on the other side's columns. Both halves reach ``IE_JOIN``. +Both halves partition on the chromosome ``INTERSECT``, since a chromosome +present on one side only yields no pairs. What completes the union is what +the unmatched half adds on top: one non-partitioned branch for the +preserved side's chromosomes the other table lacks, so rows there still +surface with NULLs without a per-chromosome branch apiece. + +A third branch completes it. Both partitions are built from ``SELECT +DISTINCT ``, and a NULL chromosome renders as a NULL literal that +``string_agg`` skips — so neither half emits a branch for it, and rows whose +chromosome is NULL would be dropped by the very join that exists to preserve +them. Those rows can never match (the partition is an equality, and ``NULL = +x`` is never true), so the emission unions them in directly, NULL-filled on +the other side. That is one extra scan of the preserved side, which DuckDB +normally prunes to nothing on the strength of null statistics: measured at +0.25% of total runtime on a base table and 2% on a CSV-backed view, and +falling as a share as the join gets more expensive. It is cheap enough +that folding it into the unmatched half would not repay the complexity. + +Emitting a per-chromosome ``LEFT JOIN`` instead — the obvious alternative — +is *not* viable: DuckDB reports ``IE_JOIN`` for that plan, yet runs it in +about 0.04s at 100,000 rows per side and fails to finish within 300s at +4,194,304, where the decomposition returns in about two seconds. Plan +inspection alone does not distinguish the two; only execution at scale +does. + +``FULL OUTER`` has no two-half form here and still takes the naive plan, as +does any outer join carrying a star or expression projection, a top-level +modifier (``GROUP BY`` / ``HAVING`` / ``ORDER BY`` / ``LIMIT`` / ``OFFSET`` +/ ``DISTINCT``), a ``WHERE`` clause, a residual predicate in its ``ON``, a +self-join, a ``TABLESAMPLE`` on either operand, or two preserved-side +projections whose output names differ only in case. + +.. note:: + + The star projection is the caveat most likely to surprise. The idiomatic + bedtools ``-loj`` translation — ``SELECT a.*, b.name`` or ``SELECT a.*, + b.*`` — is a star projection, so it declines and ``dialect="duckdb"`` + changes nothing for it. GIQL does not read table schemas, so it cannot + enumerate a star into the side-attributable columns the decomposition + needs (#202). Spell the columns out to take the fast path. + +On top of the INNER / SEMI / ANTI core shape the dialect also absorbs +several common decorations (the outer-join decomposition declines all of +them): - **Outer modifiers.** ``ORDER BY`` / ``LIMIT`` / ``OFFSET`` / ``DISTINCT`` on the outer query are appended to the outer SELECT; @@ -464,11 +544,14 @@ SQL. The soft-fallback shapes are: -- **Outer joins.** ``LEFT`` / ``RIGHT`` / ``FULL`` ``JOIN ... ON ... - INTERSECTS ...`` falls back to the naive-predicate plan. The same applies - when the join keeps its side modifier and the ``INTERSECTS`` lives in - the top-level ``WHERE`` (e.g. ``LEFT JOIN ... ON TRUE WHERE - a.interval INTERSECTS b.interval``). +- **Outer joins.** ``FULL OUTER JOIN ... ON ... INTERSECTS ...`` falls back + to the naive-predicate plan; ``LEFT`` / ``RIGHT`` are decomposed onto the + IEJoin path instead (see **Outer joins** above), except for the shapes + listed there. A fallback also applies to any outer join that keeps its + side modifier while the ``INTERSECTS`` lives in the top-level ``WHERE`` + (e.g. ``LEFT JOIN ... ON TRUE WHERE a.interval INTERSECTS b.interval``) — + there the filter discards the very unmatched rows the outer join + preserves, so it is a different query. - **SEMI / ANTI join with the INTERSECTS in the WHERE.** A ``SEMI`` / ``ANTI`` join whose column-to-column ``INTERSECTS`` sits in the top-level ``WHERE`` rather than its own ``ON`` (e.g. ``ANTI JOIN ... ON diff --git a/src/giql/expanders/_per_chrom.py b/src/giql/expanders/_per_chrom.py index be5f96b..ed1a585 100644 --- a/src/giql/expanders/_per_chrom.py +++ b/src/giql/expanders/_per_chrom.py @@ -19,37 +19,67 @@ from __future__ import annotations -from uuid import uuid4 +import hashlib #: A DuckDB expression that renders the ``string_agg`` partition's ``chrom`` #: column as a single-quoted, injection-safe SQL string literal, for #: interpolation into a per-chromosome ``WHERE chrom = ''`` filter. CHROM_LITERAL = "'''' || replace(chrom, '''', '''''') || ''''" +#: Partition-count ceiling above which the per-chromosome rewrite degrades to a +#: single non-partitioned query. The rewrite emits one ``UNION ALL`` branch per +#: distinct chromosome, so its cost scales with that cardinality while the plain +#: predicate's does not: the partitioned form wins by orders of magnitude on a +#: primary assembly and loses by orders of magnitude on a scaffold-level one. +#: The ceiling sits above every standard reference assembly's primary-chromosome +#: count and below the measured crossover (#95). +MAX_PER_CHROM_PARTITIONS = 256 + def sql_escape(text: str) -> str: """Return *text* with single quotes doubled for safe SQL string-literal use.""" return text.replace("'", "''") -def new_variable_name(prefix: str) -> str: - """Return a collision-resistant DuckDB session-variable name. +def _content_address(value_sql: str) -> str: + """Return the session-variable name that addresses *value_sql*. + + The name is a digest of the variable's own rendered value, so re-transpiling + the same query shape rebinds the same name to a byte-identical value instead + of minting a fresh one. That bounds a session's variable set by the number of + distinct query shapes rather than the number of calls -- which matters + because session variables live for the connection and the emitted script + cannot release them: the final statement has to be the ``SELECT`` for DuckDB + to return its result. - The full uuid4 hex (128 bits) keeps the name unique even across many - ``transpile()`` calls interleaved in one DuckDB session -- session variables - are global session state, so a token collision would silently rebind a - wrapper query to a different dynamic relation. + Addressing the *rendered value* rather than the inputs that built it is what + makes the property hold by construction. Hashing an argument list would + silently lose the property the day an argument is added and not added to the + digest, and nothing in the public surface can vary those arguments + independently enough for a test to catch it. + + Uniqueness is what the previous ``uuid4`` naming bought (#169), and 128 bits + of SHA-256 preserves it: two different values still get different names, so + outputs from many ``transpile()`` calls stay safe to interleave in one + session. Two *identical* values now share a name, where the rebind is a no-op. """ - return f"{prefix}_{uuid4().hex}" + return f"__giql_iejoin_{hashlib.sha256(value_sql.encode()).hexdigest()[:32]}" def set_variable_statement( - var_name: str, per_chrom_template: str, chrom_partition: str, empty_schema: str, -) -> str: - """Build the ``SET VARIABLE`` statement that assembles the per-chrom dynamic SQL. + unpartitioned_query: str, + *, + tail_branch: str | None = None, + max_partitions: int = MAX_PER_CHROM_PARTITIONS, +) -> tuple[str, str]: + """Return ``(variable name, SET VARIABLE statement)`` for the per-chrom dynamic SQL. + + The name is minted here rather than by the caller, and derived from the + rendered value rather than from these parameters, so it cannot drift from the + content it addresses. *per_chrom_template* is a SQL string-concat expression that ``string_agg`` evaluates once per row of *chrom_partition* (a ``SELECT DISTINCT `` @@ -57,16 +87,47 @@ def set_variable_statement( When no chromosome passes the partition the aggregate is ``NULL``, so it falls back to *empty_schema* -- a ``... WHERE FALSE`` query that lets DuckDB resolve every output column to its real declared type. + + *tail_branch*, when given, is a non-partitioned branch always unioned after + the per-chromosome ones, and emitted alone when no chromosome passes the + partition. It supersedes *empty_schema*, which a tail branch makes + unreachable: the tail is itself a real relation over the source table, so it + resolves every output column's declared type on its own. + + *unpartitioned_query* is the same result expressed as one query with the + chromosome equality inlined rather than partitioned out. It is selected at + execution time when the partition holds more than *max_partitions* rows, + because branch count -- not row count -- is what makes the partitioned form + expensive, and the partition's cardinality is a data property no + transpile-time check can see (#95). """ - return ( - f"SET VARIABLE {var_name} = COALESCE((\n" - f" SELECT string_agg(\n" - f" {per_chrom_template},\n" - f" ' UNION ALL '\n" - f" )\n" - f" FROM ({chrom_partition})\n" - f"), '{sql_escape(empty_schema)}')" + partition_count = f"(SELECT COUNT(*) FROM ({chrom_partition}))" + aggregated = ( + f"(\n" + f" SELECT string_agg(\n" + f" {per_chrom_template},\n" + f" ' UNION ALL '\n" + f" )\n" + f" FROM ({chrom_partition})\n" + f" )" + ) + if tail_branch is None: + partitioned = f"COALESCE({aggregated}, '{sql_escape(empty_schema)}')" + else: + partitioned = ( + f"COALESCE({aggregated} || ' UNION ALL ', '') || '{sql_escape(tail_branch)}'" + ) + # Built before the name, and referencing no part of it, so the digest below + # provably covers everything that determines the variable's value. + value_sql = ( + f"CASE\n" + f" WHEN {partition_count} > {max_partitions}\n" + f" THEN '{sql_escape(unpartitioned_query)}'\n" + f" ELSE {partitioned}\n" + f"END" ) + var_name = _content_address(value_sql) + return var_name, f"SET VARIABLE {var_name} = {value_sql}" def dynamic_relation(var_name: str, alias: str) -> str: diff --git a/src/giql/expanders/intersects.py b/src/giql/expanders/intersects.py index 938587b..11e330c 100644 --- a/src/giql/expanders/intersects.py +++ b/src/giql/expanders/intersects.py @@ -19,10 +19,13 @@ DuckDB overrides the column-to-column *join* case with its per-chromosome IEJoin plan, registered as the ``(DuckDBTarget, Intersects)`` expander in :mod:`giql.expanders.intersects_duckdb` (#169) — a pass-3 override, not a pre-pass. -That override reuses this module's :func:`_expand_spatial_op` for every shape it -declines (LEFT/RIGHT/FULL, self-joins, multiple INTERSECTS, …) and for literal-range -or residual INTERSECTS *predicates*, so the naive predicate here is the universal -fallback on DuckDB too. +For every shape it declines (FULL OUTER, the LEFT / RIGHT shapes its outer-join +decomposition declines, self-joins, multiple INTERSECTS, …) and for literal-range +or residual INTERSECTS *predicates*, that override defers to whichever expander +serves ``(GenericTarget, Intersects)``, resolved through the registry. By default +that is this module's expander, so the naive predicate here is the universal +fallback on DuckDB too; a user override registered on the generic slot reaches +those shapes as well, rather than applying under ``dialect=None`` alone. The INTERSECTS / CONTAINS / WITHIN / set-predicate expanders are registered only for :class:`~giql.targets.GenericTarget`: spatial-predicate *emission* is portable diff --git a/src/giql/expanders/intersects_duckdb.py b/src/giql/expanders/intersects_duckdb.py index f5bbf09..5c70069 100644 --- a/src/giql/expanders/intersects_duckdb.py +++ b/src/giql/expanders/intersects_duckdb.py @@ -7,10 +7,13 @@ :func:`expand_intersects_duckdb` wires it into the operator-expander registry so pass 3 (:class:`giql.expander.ExpandOperators`) dispatches DuckDB's ``Intersects`` nodes here. For the shapes it declines, and for every literal-range or residual -``Intersects`` predicate, the override defers to the shared naive overlap -predicate (:func:`giql.expanders.intersects._expand_spatial_op`) that -``dialect=None`` / ``"datafusion"`` emit on every target — a plain ``ON`` condition -the engine plans as a range join (#167). +``Intersects`` predicate, the override defers to whichever expander serves +``(GenericTarget, Intersects)`` — by default the shared naive overlap predicate +that ``dialect=None`` / ``"datafusion"`` emit on every target, a plain ``ON`` +condition the engine plans as a range join (#167). Resolving that through the +registry rather than calling the built-in directly is what lets a user's +``(GenericTarget, Intersects)`` override reach the shapes this target declines, +instead of being honoured under ``dialect=None`` and silently ignored here. Because the IEJoin rewrite emits a *whole-query* multi-statement string (``SET VARIABLE ...; SELECT ... FROM query(getvariable(...))``) rather than a @@ -35,11 +38,11 @@ from giql.constants import DEFAULT_CHROM_COL from giql.constants import DEFAULT_END_COL from giql.constants import DEFAULT_START_COL +from giql.expander import REGISTRY from giql.expander import ExpansionContext from giql.expander import register from giql.expanders._per_chrom import CHROM_LITERAL from giql.expanders._per_chrom import dynamic_relation -from giql.expanders._per_chrom import new_variable_name from giql.expanders._per_chrom import set_variable_statement from giql.expanders.intersects import SPATIAL_PREDICATE_META from giql.expanders.intersects import _expand_spatial_op @@ -49,6 +52,13 @@ from giql.expressions import Within from giql.table import Tables from giql.targets import DuckDBTarget +from giql.targets import GenericTarget + +#: Which side of an outer join keeps its unmatched rows. Narrowed by explicit +#: per-branch assignment rather than a cast: nothing type-checks this package, so +#: a cast asserts without verifying, and the value decides whether the emission +#: swaps the FROM and joined tables. +JoinSide = Literal["left", "right"] class _UnqualifiedProjectionError(Exception): @@ -82,8 +92,8 @@ class _DeclineIEJoin(Exception): IEJoin projection rebuild cannot express it (an expression, a window aggregate, a ``FILTER`` clause, a scalar subquery, an aggregate nested in an expression, or a star nested in an aggregate argument — #204, #205). - Caught in :meth:`IntersectsDuckDBIEJoinTransformer.transform_to_sql`, which - returns ``None`` so the generic naive-predicate plan handles the query, + Caught in :meth:`IntersectsDuckDBIEJoinTransformer.transform_to_parts`, + which returns ``None`` so the generic naive-predicate plan handles the query, keeping ``dialect="duckdb"`` consistent with every other backend. """ @@ -116,16 +126,24 @@ def _find_column_intersects_in(expr: exp.Expression) -> Intersects | None: ) -def _normalize_alias(name: str, *, quoted: bool = False) -> str: - """Return *name* case-folded unless it was a quoted identifier. +def _fold_identifier(name: str) -> str: + """Return *name* case-folded, matching how DuckDB compares identifiers. + + DuckDB resolves identifiers case-insensitively, and unlike the SQL standard + it does so even when they are quoted: given ``SELECT 1 AS "x", 2 AS "X"``, + both ``"x"`` and ``"X"`` select the first column. Comparing identifiers with + raw Python equality would therefore reject mixed-case queries the engine + itself accepts. - DuckDB (and standard SQL) treat unquoted identifiers case-insensitively. - Comparing aliases via raw Python equality rejects valid mixed-case queries - that the naive-predicate plan accepts; normalize via :py:meth:`str.casefold` so the - dialect matches DuckDB's identifier semantics. Quoted identifiers stay - case-sensitive per the SQL standard. + Folding quoted identifiers too is what makes the duplicate-name gate in + :func:`_match_outer_join_decomposition` correct: that gate exists because + ``AS x`` alongside ``AS X`` would otherwise bind both positions to whichever + column came first. + + Named for identifiers rather than aliases because it is applied to all of + them -- table aliases, column qualifiers, and projection output names. """ - return name if quoted else name.casefold() + return name.casefold() def _has_outer_join_intersects(query: exp.Select) -> bool: @@ -139,9 +157,14 @@ def _has_outer_join_intersects(query: exp.Select) -> bool: drop the outer-join's unmatched-row guarantee in the second shape, so we fall back whenever an outer-join is present and an INTERSECTS sits anywhere it could influence the join result. + + Since #95 this is no longer the first gate an outer join meets: + :func:`_match_outer_join_decomposition` runs ahead of it and claims the + LEFT / RIGHT shapes it can decompose. What reaches here is therefore + ``FULL OUTER``, the ``WHERE``-INTERSECTS shape above (where the filter + would discard the very rows the join preserves), and every LEFT / RIGHT + shape the decomposition declined. """ - if not isinstance(query, exp.Select): - return False outer_joins = [ join for join in query.args.get("joins") or [] if join.args.get("side") ] @@ -176,8 +199,6 @@ def _has_left_only_join_where_intersects(query: exp.Select) -> bool: that is a harmless over-decline (it only ever routes to the correct naive plan) and such shapes already decline via other gates. """ - if not isinstance(query, exp.Select): - return False has_left_only_join = any( (join.args.get("kind") or "").upper() in ("SEMI", "ANTI") for join in query.args.get("joins") or [] @@ -188,6 +209,19 @@ def _has_left_only_join_where_intersects(query: exp.Select) -> bool: return bool(where and _find_column_intersects_in(where.this)) +def _unwrap_projection_target(sel: exp.Expression) -> exp.Expression: + """Return the expression a SELECT item projects, past its alias and parens. + + Peeling both means a paren-wrapped projection such as ``(SUM(a.score)) AS s`` + is classified identically to its bare form, rather than falling through to + whatever generic branch handles an unrecognized node. + """ + target = sel.this if isinstance(sel, exp.Alias) else sel + while isinstance(target, exp.Paren): + target = target.this + return target + + def _has_star_projection(query: exp.Select) -> bool: """Return True if the SELECT list contains any star projection. @@ -204,12 +238,8 @@ def _has_star_projection(query: exp.Select) -> bool: plan expands it against DuckDB's live schema at bind time, keeping the projection identical across backends. """ - if not isinstance(query, exp.Select): - return False for sel in query.expressions: - target = sel.this if isinstance(sel, exp.Alias) else sel - while isinstance(target, exp.Paren): - target = target.this + target = _unwrap_projection_target(sel) if isinstance(target, exp.Star): return True if isinstance(target, exp.Column) and isinstance(target.this, exp.Star): @@ -286,7 +316,7 @@ def _projection_declines_to_naive( node = node.parent if in_subquery: continue - tbl = _normalize_alias(col.table) if col.table else "" + tbl = _fold_identifier(col.table) if col.table else "" if tbl not in (l_alias, r_alias) or (is_left_only and tbl == r_alias): return False return True @@ -319,12 +349,10 @@ def _count_column_intersects(query: exp.Select) -> int: Walks the full AST so the dialect's "exactly one INTERSECTS" gate cannot be circumvented by an INTERSECTS hidden inside a subquery (e.g. inside a SELECT-list scalar subquery or inside ``EXISTS (SELECT ...)``). The - SELECT-list / modifier-clause subquery gates in ``transform_to_sql`` + SELECT-list / modifier-clause subquery gates in ``transform_to_parts`` already reject those shapes, but this widened count is defense-in-depth against future gate relaxations. """ - if not isinstance(query, exp.Select): - return 0 return sum(1 for n in query.find_all(Intersects) if _is_column_intersects(n)) @@ -332,7 +360,7 @@ def _count_column_intersects(query: exp.Select) -> int: class _IEJoinSides: """Resolved two-sided join shape for the DuckDB IEJoin dialect path. - Bundles the left/right user aliases (normalized via :func:`_normalize_alias`) + Bundles the left/right user aliases (normalized via :func:`_fold_identifier`) with the un-aliased :class:`~sqlglot.expressions.Table` AST nodes and a ``kind`` discriminator identifying the join variant. The hardcoded inner aliases ``a`` and ``b`` used inside each per-chromosome subquery are @@ -376,9 +404,9 @@ def from_intersects( join_table = join.this if not isinstance(join_table, exp.Table): return None - from_alias = _normalize_alias(from_table.alias_or_name) - left_alias = _normalize_alias(intersects.this.table) - right_alias = _normalize_alias(intersects.expression.table) + from_alias = _fold_identifier(from_table.alias_or_name) + left_alias = _fold_identifier(intersects.this.table) + right_alias = _fold_identifier(intersects.expression.table) if left_alias == from_alias: l_user_alias = from_alias l_table = from_table @@ -395,10 +423,13 @@ def from_intersects( else: return None raw_kind = (join.args.get("kind") or "").upper() + kind: Literal["INNER", "SEMI", "ANTI"] if raw_kind in ("", "CROSS"): - kind: Literal["INNER", "SEMI", "ANTI"] = "INNER" - elif raw_kind in ("SEMI", "ANTI"): - kind = raw_kind # type: ignore[assignment] + kind = "INNER" + elif raw_kind == "SEMI": + kind = "SEMI" + elif raw_kind == "ANTI": + kind = "ANTI" else: return None return cls( @@ -410,26 +441,144 @@ def from_intersects( ) +def _resolve_two_base_tables( + query: exp.Select, +) -> tuple[exp.Join, exp.Table, exp.Table] | None: + """Return *query*'s single join and its two base-table operands, or ``None``. + + Accepts only a one-join query whose FROM and joined operands are both named + base tables. A GIQL table-function operand such as ``DISJOIN(genes)`` parses + as an :class:`~sqlglot.expressions.Table` with an empty ``name``, which would + interpolate an empty identifier into broken SQL, so an unnamed operand + declines here rather than downstream. (A CTE-named operand is caught by the + ``with_`` guard; an unregistered base table is fine -- the dialect uses + default columns exactly as the naive-predicate path does.) + + Every rewrite in this module has to name both relations before it can emit + anything, so these gates precede all of them. + + What they deliberately do not cover is alias distinctness. The callers + disagree about which aliases to compare -- syntactic FROM/JOIN position here + versus INTERSECTS-operand orientation in :meth:`transform_to_parts` -- and + those differ for a query like ``FROM peaks a JOIN genes b ON a.interval + INTERSECTS c.interval``, so folding the comparison in would change which + shapes decline. + """ + joins = query.args.get("joins") or [] + if len(joins) != 1: + return None + the_join = joins[0] + + from_node = query.args.get("from_") + from_table = from_node.this if from_node else None + join_table = the_join.this + if not isinstance(from_table, exp.Table) or not from_table.name: + return None + if not isinstance(join_table, exp.Table) or not join_table.name: + return None + return the_join, from_table, join_table + + +@dataclass(frozen=True) +class _IntersectsJoin: + """A two-base-table join whose ``ON`` is exactly one column-to-column INTERSECTS. + + The resolved form the two shape matchers share. ``from_table`` and + ``join_table`` keep their aliases, since the emissions render them into + ``FROM`` clauses whose columns resolve through those aliases; + ``from_alias`` and ``join_alias`` are the case-folded forms used to + attribute each projected column to a side. + """ + + side: JoinSide + from_table: exp.Table + join_table: exp.Table + from_alias: str + join_alias: str + + @classmethod + def from_query( + cls, query: exp.Select, allowed_sides: tuple[JoinSide, ...] + ) -> "_IntersectsJoin | None": + """Return *query*'s single resolved INTERSECTS join, or ``None``. + + Builds on :func:`_resolve_two_base_tables` and adds the gates specific to + a bare outer-join ``ON``: the accepted side, a plain join kind, exactly + one column-to-column INTERSECTS with no residual, and two distinct + aliases. Shared by the two shape matchers; the INNER / SEMI / ANTI path + in :meth:`IntersectsDuckDBIEJoinTransformer.transform_to_parts` accepts + shapes these gates reject and so builds on the base-table helper + directly. + + *allowed_sides* entries must already be case-folded, since the join's own + side is folded before the membership test. + """ + resolved = _resolve_two_base_tables(query) + if resolved is None: + return None + the_join, from_table, join_table = resolved + + # Assigned per branch rather than cast, so an unexpected side declines + # here instead of reaching the emission as an unhandled value. + raw_side = _fold_identifier(the_join.args.get("side") or "") + side: JoinSide + if raw_side == "left": + side = "left" + elif raw_side == "right": + side = "right" + else: + return None + if side not in allowed_sides: + return None + if (the_join.args.get("kind") or "").upper() not in ("", "OUTER"): + return None + + on = the_join.args.get("on") + if on is None: + return None + intersects = _find_column_intersects_in(on) + if intersects is None or _count_column_intersects(query) != 1: + return None + # v1 supports only a bare INTERSECTS ON. A residual would have to be + # applied identically to every relation a composed rewrite emits -- the + # INNER half's join and the unmatched half's NOT EXISTS correlation -- + # which none of them coordinate yet. + if _strip_intersects(on, intersects) is not None: + return None + + from_alias = _fold_identifier(from_table.alias_or_name) + join_alias = _fold_identifier(join_table.alias_or_name) + if from_alias == join_alias: + return None + + return cls( + side=side, + from_table=from_table, + join_table=join_table, + from_alias=from_alias, + join_alias=join_alias, + ) + + @dataclass(frozen=True) class _CountOverlapsShape: """The matched ``count_overlaps`` LEFT-join-with-COUNT shape (#209). - Bundles the LEFT :class:`~sqlglot.expressions.Join`, the un-aliased left - (FROM) base table, the left/right user aliases (case-folded), the left-side - group-key SELECT items, and the single ``COUNT`` aggregate SELECT item. The - emission reuses the INNER path to build the per-chromosome IEJoin count and - wraps it in a zero-fill LEFT join against the distinct left keys. + Bundles the left (FROM) base table, the left-side group-key SELECT items, + and the single ``COUNT`` aggregate SELECT item. The emission reuses the + INNER path to build the per-chromosome IEJoin count and wraps it in a + zero-fill LEFT join against the distinct left keys. + + ``left_table`` keeps its alias: the distinct-key relation renders it + directly and the group-key projections resolve through that alias. """ - the_join: exp.Join left_table: exp.Table - left_alias: str - right_alias: str key_items: tuple[exp.Expression, ...] agg_item: exp.Alias -def _match_count_overlaps(query: exp.Expression) -> "_CountOverlapsShape | None": +def _match_count_overlaps(query: exp.Select) -> "_CountOverlapsShape | None": """Return the count_overlaps shape when *query* matches it, else ``None``. Matches exactly ``SELECT , COUNT() AS FROM @@ -440,8 +589,6 @@ def _match_count_overlaps(query: exp.Expression) -> "_CountOverlapsShape | None" non-LEFT outer join, extra ON residuals) returns ``None`` so the caller keeps the existing behaviour (the naive-predicate plan). """ - if not isinstance(query, exp.Select): - return None if query.args.get("with_") or query.args.get("distinct"): return None if any( @@ -451,42 +598,17 @@ def _match_count_overlaps(query: exp.Expression) -> "_CountOverlapsShape | None" if query.args.get("group") is None or query.args.get("where") is not None: return None - joins = query.args.get("joins") or [] - if len(joins) != 1: - return None - the_join = joins[0] - if _normalize_alias(the_join.args.get("side") or "") != "left": - return None - if (the_join.args.get("kind") or "").upper() not in ("", "OUTER"): - return None - on = the_join.args.get("on") - if on is None: - return None - intersects = _find_column_intersects_in(on) - if intersects is None or _count_column_intersects(query) != 1: - return None - # v1 supports only a bare INTERSECTS ON (no residual join conditions). - if _strip_intersects(on, intersects) is not None: - return None - - from_node = query.args.get("from_") - from_table = from_node.this if from_node else None - join_table = the_join.this - if not isinstance(from_table, exp.Table) or not from_table.name: - return None - if not isinstance(join_table, exp.Table) or not join_table.name: - return None - left_alias = _normalize_alias(from_table.alias_or_name) - right_alias = _normalize_alias(join_table.alias_or_name) - if left_alias == right_alias: + join = _IntersectsJoin.from_query(query, ("left",)) + if join is None: return None + from_table = join.from_table + left_alias = join.from_alias + right_alias = join.join_alias key_items: list[exp.Expression] = [] agg_item: exp.Alias | None = None for sel in query.expressions: - target = sel.this if isinstance(sel, exp.Alias) else sel - while isinstance(target, exp.Paren): - target = target.this + target = _unwrap_projection_target(sel) if isinstance(target, exp.AggFunc): # Exactly one COUNT() / COUNT(DISTINCT ) aggregate, # aliased so the wrapper can reference and re-project it. @@ -502,7 +624,7 @@ def _match_count_overlaps(query: exp.Expression) -> "_CountOverlapsShape | None" if not ( isinstance(arg, exp.Column) and not isinstance(arg.this, exp.Star) - and _normalize_alias(arg.table) == right_alias + and _fold_identifier(arg.table) == right_alias ): return None agg_item = sel @@ -511,7 +633,7 @@ def _match_count_overlaps(query: exp.Expression) -> "_CountOverlapsShape | None" and target.table and not isinstance(target.this, exp.Star) ): - if _normalize_alias(target.table) != left_alias: + if _fold_identifier(target.table) != left_alias: return None key_items.append(sel) else: @@ -539,19 +661,155 @@ def _match_count_overlaps(query: exp.Expression) -> "_CountOverlapsShape | None" return None return _CountOverlapsShape( - the_join=the_join, left_table=from_table, - left_alias=left_alias, - right_alias=right_alias, key_items=tuple(key_items), agg_item=agg_item, ) +@dataclass(frozen=True) +class _OuterJoinShape: + """A LEFT / RIGHT outer-join ``INTERSECTS`` decomposable into two halves (#95). + + Carries the resolved join plus the SELECT-list positions belonging to each + side. ``side`` is ``"left"`` or ``"right"``; for ``"right"`` the emission + first swaps the FROM and joined tables so one LEFT-shaped code path serves + both orientations. + + The row-preserving side is derived from the join rather than copied out of + it, so the table node and the alias cannot drift apart. ``preserved_table`` + keeps its alias intact -- the NULL-chromosome branch selects from it + directly and references its columns through that alias, so stripping the + alias would leave that branch unable to bind. + + ``preserved_indexes`` and ``null_indexes`` partition the projection by + position: the preserved side's columns survive into both halves, while the + other side's become NULL fills in the unmatched half. + """ + + join: _IntersectsJoin + preserved_indexes: tuple[int, ...] + null_indexes: tuple[int, ...] + + @property + def side(self) -> JoinSide: + """Which side keeps its unmatched rows.""" + return self.join.side + + @property + def preserved_table(self) -> exp.Table: + """The row-preserving side's table node, alias intact.""" + return self.join.from_table if self.side == "left" else self.join.join_table + + @property + def preserved_alias(self) -> str: + """The row-preserving side's case-folded alias.""" + return self.join.from_alias if self.side == "left" else self.join.join_alias + + +def _match_outer_join_decomposition(query: exp.Select) -> "_OuterJoinShape | None": + """Return the decomposable outer-join shape for *query*, else ``None``. + + Matches ``SELECT FROM a LEFT|RIGHT JOIN + b ON `` -- the shape a ``UNION ALL`` + of an INNER half and an unmatched-rows half reproduces exactly (#95). + + Everything else returns ``None`` so the caller keeps the naive-predicate + plan. In particular: + + - ``FULL OUTER`` preserves rows on both sides and has no two-half form here. + - An ``INTERSECTS`` in the top-level ``WHERE`` under an outer join is a + different query -- the filter drops the very unmatched rows the outer join + preserves -- so it must keep declining, as must any other ``WHERE`` + residual this rewrite does not yet thread through both halves. + - Top-level modifiers (``GROUP BY`` / ``HAVING`` / ``ORDER BY`` / ``LIMIT`` + / ``OFFSET`` / ``DISTINCT``) belong on the union, but the recursive + transpile of each half would apply them per branch instead. + - A projection that is not a side-attributable qualified column (a star, an + expression, an aggregate) cannot be split into "survives" and "becomes + NULL", which is exactly what the unmatched half needs. This is also why + the ``bedtools -wao`` ``CASE`` projection still declines until #109 lands. + """ + # Whitelist, not a blocklist: the rewrite re-emits the query as a UNION ALL + # of two independently transpiled halves, so ANY top-level clause it does + # not itself read would be applied per-half instead of over the union -- + # silently changing the answer. Enumerating clauses to *reject* leaves the + # rewrite exposed to every clause sqlglot grows later, which is exactly how + # ``QUALIFY`` slipped onto the fast path and had its filter dropped. Reject + # anything outside the set ``_build_outer_join_parts`` actually consumes. + if any( + value is not None and value != [] + for key, value in query.args.items() + if key not in ("expressions", "from_", "joins") + ): + return None + + join = _IntersectsJoin.from_query(query, ("left", "right")) + if join is None: + return None + # A TABLESAMPLE hangs off the table node rather than the top-level SELECT, + # so the clause whitelist above cannot see it. Each half would sample the + # relation independently, making the union neither a superset nor a subset + # of a real outer join over one sample -- and the sample clause lands in a + # position the per-chromosome template cannot even parse. + if join.from_table.args.get("sample") is not None: + return None + if join.join_table.args.get("sample") is not None: + return None + + if join.side == "left": + preserved_alias, other_alias = join.from_alias, join.join_alias + else: + preserved_alias, other_alias = join.join_alias, join.from_alias + + if not query.expressions: + return None + preserved_indexes: list[int] = [] + null_indexes: list[int] = [] + for idx, sel in enumerate(query.expressions): + target = _unwrap_projection_target(sel) + if ( + not isinstance(target, exp.Column) + or isinstance(target.this, exp.Star) + or not target.table + ): + return None + tbl = _fold_identifier(target.table) + if tbl == preserved_alias: + preserved_indexes.append(idx) + elif tbl == other_alias: + null_indexes.append(idx) + else: + return None + + # The unmatched half re-projects the preserved columns out of a subquery by + # their output names, so duplicates there would bind ambiguously. The + # comparison must be case-INSENSITIVE: DuckDB resolves identifiers that way + # even when they are quoted, so ``AS x`` alongside ``AS X`` would otherwise + # pass this gate and then bind both positions to whichever column came + # first -- returning the wrong value and, because the branch's type then + # wins the UNION ALL unification, corrupting the matched rows too. + # Deliberately preserved-side only: duplicate names among the NULL-filled + # positions are never re-projected by name, so they stay supported. + preserved_names = [ + _fold_identifier(query.expressions[i].output_name) for i in preserved_indexes + ] + if len(set(preserved_names)) != len(preserved_names): + return None + + return _OuterJoinShape( + join=join, + preserved_indexes=tuple(preserved_indexes), + null_indexes=tuple(null_indexes), + ) + + class IntersectsDuckDBIEJoinTransformer: """Transform column-to-column INTERSECTS joins into a DuckDB IEJoin pattern. - Emits a two-step dynamic SQL output: + Emits dynamic SQL in two steps per declared session variable — one + variable for the INNER / SEMI / ANTI shapes, two for a decomposed outer + join: 1. ``SET VARIABLE __giql_iejoin_ = COALESCE(, );`` where ```` is a ``string_agg`` over the chromosome @@ -565,10 +823,11 @@ class IntersectsDuckDBIEJoinTransformer: Each per-chromosome subquery uses pure inequality predicates which DuckDB plans through its range-join family (``IE_JOIN`` or ``PIECEWISE_MERGE_JOIN``). - The dialect supports INNER, SEMI, and ANTI variants. Crucially, DuckDB only - selects ``IE_JOIN`` for INNER pure-inequality joins — a bare ``SEMI JOIN`` / - ``ANTI JOIN`` inequality join is planned as a ``BLOCKWISE_NL_JOIN`` (a nested - loop, quadratic). So the left-only shapes are emitted as a correlated + The dialect supports INNER, SEMI, and ANTI variants, plus LEFT / RIGHT + outer joins. Crucially, DuckDB only selects ``IE_JOIN`` for INNER + pure-inequality joins — a bare ``SEMI JOIN`` / ``ANTI JOIN`` inequality join + is planned as a ``BLOCKWISE_NL_JOIN`` (a nested loop, quadratic). So the + left-only shapes are emitted as a correlated ``WHERE EXISTS`` (SEMI) / ``WHERE NOT EXISTS`` (ANTI) subquery instead of a semi/anti join keyword, which reaches ``IE_JOIN`` while preserving exact semantics (#208). SEMI / ANTI restrict the outer SELECT to left-side @@ -602,9 +861,18 @@ class IntersectsDuckDBIEJoinTransformer: (wrapped to :class:`ValueError` at the public boundary). A right-side ``b.*`` under SEMI / ANTI declines to the naive plan with every other star (which then rejects the out-of-scope right table at bind time). - - ``ANTI JOIN`` partitions on the left table's distinct chromosomes - only (not the chromosome INTERSECT used by INNER / SEMI) so that - left rows on chromosomes absent from the right table are preserved. + - ``ANTI JOIN`` partitions on the chromosome INTERSECT like INNER and + SEMI, then unions one non-partitioned branch carrying the left rows + whose chromosome the right table lacks. Those rows cannot match, so a + branch apiece would scan both tables to prove an emptiness the + partition already knows (#95). + - ``LEFT`` / ``RIGHT`` outer joins are not emitted as outer joins at all: + DuckDB's ``IE_JOIN`` is INNER-only, so the query is decomposed into an + INNER half and a ``NOT EXISTS`` unmatched half combined with + ``UNION ALL``, plus a third branch carrying the preserved side's + NULL-chromosome rows, which no per-chromosome partition can reach + (#95). ``RIGHT`` is served by swapping the FROM and joined tables so a + single LEFT-shaped path covers both orientations. """ def __init__(self, tables: Tables): @@ -616,7 +884,36 @@ def __init__(self, tables: Tables): self.tables = tables def transform_to_sql(self, query: exp.Expression) -> str | None: - """Return DuckDB IEJoin SQL for *query* if applicable, else None. + """Return DuckDB IEJoin SQL for *query* if applicable, else ``None``. + + Renders what :meth:`transform_to_parts` returns as one script. Both + reach the same primitive: a rewrite composing on top of this one calls + that method instead of splitting this string, which cannot be taken + apart safely -- the separator appears inside the script itself whenever + an identifier contains it. + + The result is a multi-statement script whose ``SET VARIABLE`` setup and + final ``SELECT`` must run on one connection, in order. + + *query* is borrowed, not owned: it is never mutated. The registry entry + point passes the live statement AST that pass 3 goes on transforming, + and the naive-predicate fallback rewrites that same node in place when + this path declines, so that has to hold. + + Raises + ------ + ValueError + If a projection names a column the rewrite cannot attribute to a + side. + """ + parts = self.transform_to_parts(query) + if parts is None: + return None + setup, select = parts + return f"{setup};\n{select}" + + def transform_to_parts(self, query: exp.Expression) -> tuple[str, str] | None: + """Return ``(setup statements, final SELECT)`` for *query*, else ``None``. The dialect rewrites the *whole* query as ``SET VARIABLE __giql_iejoin_ = ...; SELECT ... FROM @@ -656,7 +953,19 @@ def transform_to_sql(self, query: exp.Expression) -> str | None: # Checked before the outer-join decline below since it *is* an outer join. count_shape = _match_count_overlaps(query) if count_shape is not None: - return self._build_count_overlaps_sql(query, count_shape) + return self._build_count_overlaps_parts(query, count_shape) + + # Outer-join decomposition (#95): a LEFT / RIGHT JOIN whose projections + # are side-attributable columns is emitted as the INNER pairs UNION ALL + # the unmatched preserved-side rows with NULL fills, so both halves reach + # IE_JOIN. Checked after count_overlaps -- also a LEFT JOIN, with its own + # faster zero-fill path -- and before the decline below, which still + # catches FULL OUTER and the WHERE-INTERSECTS shape. + outer_shape = _match_outer_join_decomposition(query) + if outer_shape is not None: + decomposed = self._build_outer_join_parts(query, outer_shape) + if decomposed is not None: + return decomposed if _has_outer_join_intersects(query): return None @@ -699,23 +1008,21 @@ def transform_to_sql(self, query: exp.Expression) -> str | None: ): return None for sel in query.expressions: - target = sel.this if isinstance(sel, exp.Alias) else sel - while isinstance(target, exp.Paren): - target = target.this + target = _unwrap_projection_target(sel) if isinstance(target, exp.AggFunc) and ( target.find(exp.Subquery) is not None or target.find(exp.Select) is not None ): return None - # The supported shape connects exactly two tables via one INTERSECTS, - # so the top level has exactly one join (either an explicit JOIN ... - # ON or an implicit comma-join in WHERE). A third comma/CROSS-joined - # table would otherwise be silently dropped by _build_sql. - joins = query.args.get("joins") or [] - if len(joins) != 1: + # The supported shape connects exactly two named base tables via one + # INTERSECTS (either an explicit JOIN ... ON or an implicit comma-join + # in WHERE). A third comma/CROSS-joined table would otherwise be + # silently dropped by _build_sql. + resolved = _resolve_two_base_tables(query) + if resolved is None: return None - the_join = joins[0] + the_join, from_table, join_table = resolved # NATURAL needs schema introspection we don't have at transpile time # (Table configs only expose chrom/start/end/strand; user columns @@ -736,31 +1043,19 @@ def transform_to_sql(self, query: exp.Expression) -> str | None: # INNER (the default), CROSS (functionally equivalent to INNER once # the chromosome partition is in place), SEMI, and ANTI all engage. - # Everything else falls back so the naive-predicate plan can preserve its - # semantics (e.g. RIGHT / FULL outer joins, which the naive overlap - # predicate expresses as a plain outer ``ON`` condition). + # Everything else falls back so the naive-predicate plan can preserve + # its semantics -- FULL OUTER, and the LEFT / RIGHT shapes the outer-join + # decomposition declined before handing the query here (#95), which the + # naive overlap predicate expresses as a plain outer ``ON`` condition. kind_str = the_join.args.get("kind") if kind_str and kind_str.upper() not in ("INNER", "CROSS", "SEMI", "ANTI"): return None - intersects, the_join = self._find_target_join(query) - if intersects is None or the_join is None: - return None - - from_table = query.args["from_"].this - if not isinstance(from_table, exp.Table): - return None - - # Reject non-base-table operands. A GIQL table-function operand such - # as ``DISJOIN(genes)`` parses as an ``exp.Table`` with an empty - # ``name`` and would be interpolated as an empty identifier into - # broken SQL. (A CTE-named operand is already handled by the - # ``with_`` guard above; an unregistered base table is fine — the - # dialect uses default columns just like the naive-predicate path does.) - if not from_table.name: - return None - join_table = the_join.this - if not isinstance(join_table, exp.Table) or not join_table.name: + # Locates which of the two relations carries the INTERSECTS, and where + # (an explicit ON, or the top-level WHERE of a comma join). The single + # join gate above means it can only return the join already resolved. + intersects, _ = self._find_target_join(query) + if intersects is None: return None sides = _IEJoinSides.from_intersects(from_table, the_join, intersects) @@ -803,56 +1098,6 @@ def transform_to_sql(self, query: exp.Expression) -> str | None: except _UnqualifiedProjectionError as exc: raise ValueError(str(exc)) from exc - def _build_count_overlaps_sql( - self, query: exp.Select, shape: "_CountOverlapsShape" - ) -> str | None: - """Render the count_overlaps zero-fill SQL for a matched LEFT-join shape (#209). - - Reuses the INNER IEJoin path: an INNER copy of the query (same - projections and GROUP BY, LEFT downgraded to INNER) is transpiled by - :meth:`transform_to_sql` into ``SET VARIABLE ...; ``, which - DuckDB plans through ``IE_JOIN`` + hash aggregate. The INNER count drops - left rows with no overlap, so its ``SELECT`` is wrapped as a CTE and - LEFT-joined back onto the distinct left keys, zero-filling the missing - counts. Returns ``None`` (fall back to the naive plan) if the INNER copy - itself declines. - """ - inner_query = query.copy() - inner_join = inner_query.args["joins"][0] - inner_join.set("side", None) - inner_join.set("kind", None) - inner_sql = self.transform_to_sql(inner_query) - if inner_sql is None or ";\n" not in inner_sql: - return None - set_var_stmt, inner_select = inner_sql.split(";\n", 1) - - q = self._sql_quote_ident - key_names = [item.output_name for item in shape.key_items] - key_projections = [item.sql(dialect="duckdb") for item in shape.key_items] - group_exprs = [ - (item.this if isinstance(item, exp.Alias) else item).sql(dialect="duckdb") - for item in shape.key_items - ] - agg_name = shape.agg_item.output_name - - # Distinct left-key relation, rendering the FROM table with its own alias - # so the key projections (``a.chrom`` ...) resolve against it. - base_cte = ( - f"SELECT {', '.join(key_projections)} " - f"FROM {shape.left_table.sql(dialect='duckdb')} " - f"GROUP BY {', '.join(group_exprs)}" - ) - using_cols = ", ".join(q(name) for name in key_names) - base_out = ", ".join(f"base.{q(name)}" for name in key_names) - wrapper = ( - f"WITH __giql_counts AS ({inner_select}), " - f"__giql_base AS ({base_cte}) " - f"SELECT {base_out}, COALESCE(c.{q(agg_name)}, 0) AS {q(agg_name)} " - f"FROM __giql_base AS base " - f"LEFT JOIN __giql_counts AS c USING ({using_cols})" - ) - return set_var_stmt + ";\n" + wrapper - @staticmethod def _sql_escape(s: str) -> str: """Return *s* with single quotes doubled for safe SQL string literal use.""" @@ -931,7 +1176,7 @@ def _validate_extra_qualifiers( valid_aliases = {sides.left_user_alias, sides.right_user_alias} for predicate in extras: for col in predicate.find_all(exp.Column): - col_table = _normalize_alias(col.table) if col.table else "" + col_table = _fold_identifier(col.table) if col.table else "" if not col_table: raise _UnqualifiedProjectionError( f"dialect='duckdb' cannot inline extra predicate " @@ -980,7 +1225,7 @@ def _rewrite_refs_for_per_chrom_subquery( def replace(n: exp.Expression) -> exp.Expression: if not isinstance(n, exp.Column): return n - col_table = _normalize_alias(n.table) if n.table else "" + col_table = _fold_identifier(n.table) if n.table else "" if col_table == sides.left_user_alias: new = n.copy() new.set("table", exp.to_identifier(sides.left_inner_alias)) @@ -1032,7 +1277,7 @@ def _rewrite_refs_for_wrapper_relation( def replace(n: exp.Expression) -> exp.Expression: if not isinstance(n, exp.Column): return n - col_table = _normalize_alias(n.table) if n.table else "" + col_table = _fold_identifier(n.table) if n.table else "" if col_table not in valid_aliases: return n key = (col_table, n.name) @@ -1048,6 +1293,205 @@ def replace(n: exp.Expression) -> exp.Expression: rewritten = node.transform(replace, copy=True) return rewritten.sql(dialect="duckdb") + def _iejoin_variant_parts( + self, query: exp.Select, *, kind: str | None + ) -> tuple[str, str] | None: + """Transpile a copy of *query* with its single join retargeted to *kind*. + + The copy is taken here rather than by the caller, so no caller ever hands + its own node to the transform. Whether the emission mutates what it is + given stops being something each composed rewrite has to know and get + right; before this, one of them defended the invariant with a comment + and a second defensive copy. + """ + variant = query.copy() + join = variant.args["joins"][0] + join.set("side", None) + join.set("kind", kind) + return self.transform_to_parts(variant) + + def _build_count_overlaps_parts( + self, query: exp.Select, shape: "_CountOverlapsShape" + ) -> tuple[str, str] | None: + """Render the count_overlaps zero-fill SQL for a matched LEFT-join shape (#209). + + Reuses the INNER IEJoin path: an INNER copy of the query (same + projections and GROUP BY, LEFT downgraded to INNER) is transpiled by + :meth:`transform_to_parts` into a setup statement and a count + ``SELECT``, which DuckDB plans through ``IE_JOIN`` + hash aggregate. The + INNER count drops left rows with no overlap, so its ``SELECT`` is + wrapped as a CTE and LEFT-joined back onto the distinct left keys, + zero-filling the missing counts. Returns ``None`` (fall back to the + naive plan) if the INNER copy itself declines. + """ + inner_parts = self._iejoin_variant_parts(query, kind=None) + if inner_parts is None: + return None + set_var_stmt, inner_select = inner_parts + + q = self._sql_quote_ident + key_names = [item.output_name for item in shape.key_items] + key_projections = [item.sql(dialect="duckdb") for item in shape.key_items] + group_exprs = [ + (item.this if isinstance(item, exp.Alias) else item).sql(dialect="duckdb") + for item in shape.key_items + ] + agg_name = shape.agg_item.output_name + + # Distinct left-key relation, rendering the FROM table with its own alias + # so the key projections (``a.chrom`` ...) resolve against it. + base_cte = ( + f"SELECT {', '.join(key_projections)} " + f"FROM {shape.left_table.sql(dialect='duckdb')} " + f"GROUP BY {', '.join(group_exprs)}" + ) + using_cols = ", ".join(q(name) for name in key_names) + base_out = ", ".join(f"base.{q(name)}" for name in key_names) + wrapper = ( + f"WITH __giql_counts AS ({inner_select}), " + f"__giql_base AS ({base_cte}) " + f"SELECT {base_out}, COALESCE(c.{q(agg_name)}, 0) AS {q(agg_name)} " + f"FROM __giql_base AS base " + f"LEFT JOIN __giql_counts AS c USING ({using_cols})" + ) + return set_var_stmt, wrapper + + def _build_outer_join_parts( + self, query: exp.Select, shape: "_OuterJoinShape" + ) -> tuple[str, str] | None: + """Render an outer join as INNER pairs UNION ALL unmatched rows (#95). + + Reuses the two IEJoin paths that already exist rather than emitting an + outer join: an INNER copy supplies the matched pairs, and an ANTI copy + (the #208 correlated ``NOT EXISTS`` rewrite) supplies the preserved-side + rows with no overlap. Both reach ``IE_JOIN``. A per-chromosome ``LEFT + JOIN`` does not, even though ``EXPLAIN`` reports ``IE_JOIN`` for it. + Measured, that plan runs in about 0.04s at 100,000 rows per side and + does not finish within 300s at 4,194,304, where this decomposition + returns in about two seconds. Plan inspection alone does not separate + them; only execution at scale does. + + Both halves partition on the chromosome ``INTERSECT``, since a + chromosome present on one side only yields no pairs. What completes the + union is what the ANTI half adds on top: one non-partitioned branch for + the preserved side's chromosomes the other side lacks, so preserved rows + there still surface with NULLs. + + Returns ``None`` (fall back to the naive-predicate plan) when either + half declines, so the query falls back as one unit. + """ + # RIGHT JOIN is the mirror of LEFT: swapping the FROM and joined tables + # rewrites ``a RIGHT JOIN b`` as the equivalent ``b LEFT JOIN a``, so a + # single code path serves both. The projections are alias-qualified and + # resolve unchanged across the swap. + work = query.copy() + if shape.side == "right": + work_join = work.args["joins"][0] + from_node = work.args["from_"] + swapped_from = work_join.this + work_join.set("this", from_node.this) + from_node.set("this", swapped_from) + + inner_parts = self._iejoin_variant_parts(work, kind=None) + if inner_parts is None: + return None + inner_setup, inner_select = inner_parts + + # The unmatched half projects the preserved side only: the other side is + # out of scope under ANTI, and a NULL literal in the SELECT list would + # itself decline the projection rebuild. The NULLs are re-inserted by the + # wrapper below instead. + preserved_config = self.tables.get(shape.preserved_table.name) + chrom_col = preserved_config.chrom_col if preserved_config else DEFAULT_CHROM_COL + + preserved_sels = [query.expressions[i].copy() for i in shape.preserved_indexes] + if not preserved_sels: + # Nothing from the preserved side is projected (``SELECT b.chrom FROM + # a LEFT JOIN b ...``). The half still needs a column to select, so + # carry the preserved side's chromosome column and drop it in the + # wrapper, which projects NULL for every original position anyway. + preserved_sels = [exp.column(chrom_col, table=shape.preserved_alias)] + # Safe to mutate in place: the INNER half above transpiled a copy, so + # nothing downstream holds a reference to ``work``. + work.set("expressions", preserved_sels) + anti_parts = self._iejoin_variant_parts(work, kind="ANTI") + if anti_parts is None: + return None + anti_setup, anti_select = anti_parts + + # Both remaining branches re-project the original SELECT order, filling + # the other side's positions with NULL. That NULL-fill rule is what makes + # the union reproduce outer-join semantics, so it is expressed once here + # rather than once per branch. DuckDB resolves each NULL's type from the + # first UNION ALL branch, so the dialect needs no schema knowledge. + # + # They differ only in how a preserved position is spelled: the unmatched + # half reads it back out of a subquery by output name, while the + # NULL-chromosome branch selects from the base table and so renders the + # original qualified column. + q = self._sql_quote_ident + preserved_names = [ + query.expressions[i].output_name for i in shape.preserved_indexes + ] + wrapper_items: list[str] = [] + null_chrom_items: list[str] = [] + preserved_seen = 0 + for idx, sel in enumerate(query.expressions): + output = q(sel.output_name) + if idx in shape.null_indexes: + null_fill = f"NULL AS {output}" + wrapper_items.append(null_fill) + null_chrom_items.append(null_fill) + continue + wrapper_items.append(f"{q(preserved_names[preserved_seen])} AS {output}") + preserved_seen += 1 + column = _unwrap_projection_target(sel) + null_chrom_items.append(f"{column.sql(dialect='duckdb')} AS {output}") + + unmatched_select = ( + f"SELECT {', '.join(wrapper_items)} FROM ({anti_select}) AS __giql_unmatched" + ) + + # A preserved-side row whose chromosome is NULL can never match: the + # per-chromosome partition is an equality on chrom and ``NULL = x`` is + # never true. Neither half can surface those rows on its own -- both + # partitions come from ``SELECT DISTINCT ``, where a NULL renders + # as a NULL literal that ``string_agg`` skips, so no branch is ever + # emitted for it. Union them in directly, or the decomposition would + # silently drop the very rows an outer join exists to preserve. + # Qualify the WHERE with the same identifier node the FROM clause renders + # rather than with the case-folded alias. The two bind to one relation + # today only because DuckDB folds identifiers -- quoted ones included -- + # so ``FROM peaks AS "A"`` happened to accept ``a.chrom``. Deriving both + # from one node removes that dependency instead of resting on it. + # + # The sibling above, which carries the preserved chromosome into the + # ANTI half, deliberately keeps the folded alias: that column is fed + # back through transform_to_parts, which matches aliases + # case-insensitively and re-emits it against the per-chromosome + # subquery's own ``a``, never against the user's alias. + alias_node = shape.preserved_table.args.get("alias") + preserved_qualifier = ( + alias_node.this if alias_node is not None else shape.preserved_table.this + ) + preserved_chrom = exp.column(chrom_col, table=preserved_qualifier) + null_chrom_select = ( + f"SELECT {', '.join(null_chrom_items)} " + f"FROM {shape.preserved_table.sql(dialect='duckdb')} " + f"WHERE {preserved_chrom.sql(dialect='duckdb')} IS NULL" + ) + + # The matched half is used as a UNION ALL branch directly. Wrapping it in + # ``SELECT * FROM (...)`` would make DuckDB de-duplicate any repeated + # output name (``chrom`` becoming ``chrom_1``), and that renaming would + # become the union's schema -- so a flag documented as a pure performance + # opt-in would silently relabel the result columns, and precisely for the + # ``a.chrom, ..., b.chrom`` projection bedtools users write. + union = ( + f"{inner_select} UNION ALL {unmatched_select} UNION ALL {null_chrom_select}" + ) + return ";\n".join([inner_setup, anti_setup]), union + def _extract_extra_predicates( self, query: exp.Select, @@ -1123,13 +1567,13 @@ def _find_target_join( from_table = query.args["from_"].this if not isinstance(from_table, exp.Table): return None, None - from_alias = _normalize_alias(from_table.alias_or_name) - left_alias = _normalize_alias(intersects.this.table) - right_alias = _normalize_alias(intersects.expression.table) + from_alias = _fold_identifier(from_table.alias_or_name) + left_alias = _fold_identifier(intersects.this.table) + right_alias = _fold_identifier(intersects.expression.table) target_alias = right_alias if left_alias == from_alias else left_alias for join in query.args.get("joins") or []: if isinstance(join.this, exp.Table): - if _normalize_alias(join.this.alias_or_name) == target_alias: + if _fold_identifier(join.this.alias_or_name) == target_alias: return intersects, join return None, None @@ -1140,8 +1584,12 @@ def _build_sql( intersects: Intersects, sides: "_IEJoinSides", the_join: exp.Join, - ) -> str | None: - """Render the multi-statement DuckDB IEJoin SQL string for *query*. + ) -> tuple[str, str] | None: + """Render the DuckDB IEJoin ``(setup statements, final SELECT)`` for *query*. + + The two halves are returned separately rather than pre-joined so a + rewrite composed on top never has to recover them by splitting rendered + SQL, which would break on an identifier containing ``";\\n"``. Returns ``None`` when a soft-fallback condition fires (the residual of the join carries a shape the naive-predicate plan can handle but the @@ -1149,7 +1597,7 @@ def _build_sql( that is not both tables' ``chrom_col``). May propagate :class:`_DeclineIEJoin` from the :meth:`_resolve_projections` projection pre-scan (a naive-valid projection the rebuild cannot express — #204, - #205); :meth:`transform_to_sql` catches it and declines to the naive + #205); :meth:`transform_to_parts` catches it and declines to the naive plan. Raises :class:`_UnqualifiedProjectionError` when a user-mistake condition fires; callers translating to the public surface wrap the raise to :class:`ValueError`. @@ -1195,10 +1643,10 @@ def _build_sql( # USING equi-join for the engine to resolve. using_cols = the_join.args.get("using") or [] if using_cols: - using_name = _normalize_alias(using_cols[0].name) + using_name = _fold_identifier(using_cols[0].name) if ( - _normalize_alias(l_chrom) != _normalize_alias(r_chrom) - or _normalize_alias(l_chrom) != using_name + _fold_identifier(l_chrom) != _fold_identifier(r_chrom) + or _fold_identifier(l_chrom) != using_name ): return None @@ -1212,8 +1660,6 @@ def _build_sql( outer_where_residuals, ) - var_name = new_variable_name("__giql_iejoin") - # Canonicalize endpoints to 0-based half-open and use strict # operators. Mixing inclusive operators with raw closed-closed # coordinates would silently report non-overlapping touching @@ -1314,23 +1760,64 @@ def _build_sql( f"FROM {l_table_ident} a, {r_table_ident} b WHERE FALSE" ) - # Partition source: INTERSECT for INNER / SEMI (a chromosome only - # on one side produces no matches anyway), left-distinct for ANTI - # (a chromosome present only on the left must still emit its rows - # under ANTI semantics). + # Partition source: the chromosomes both sides share. A chromosome + # present on only one side can never satisfy the overlap predicate, so a + # per-chromosome branch for it is wasted work -- it scans both tables to + # prove an emptiness the partition already knows. ANTI must still emit + # its left-only chromosomes' rows, but as one non-partitioned tail + # branch rather than one branch apiece (#95). + chrom_partition_subquery = ( + f"SELECT DISTINCT {q(l_chrom)} AS chrom FROM {l_table_ident} " + f"INTERSECT SELECT DISTINCT {q(r_chrom)} AS chrom " + f"FROM {r_table_ident}" + ) + + # ANTI's left-only chromosomes, in one branch. Both NULL guards are + # load-bearing: ``x NOT IN ()`` is never true, and a + # left row whose chromosome is NULL must stay out of this branch to match + # the per-chromosome path, which drops it because ``string_agg`` skips + # the NULL partition row (#224). The outer-join decomposition depends on + # that exclusion -- it emits NULL-chromosome rows from its own third + # branch and would otherwise double-count them. + tail_branch: str | None = None if sides.kind == "ANTI": - chrom_partition_subquery = ( - f"SELECT DISTINCT {q(l_chrom)} AS chrom FROM {l_table_ident}" + tail_branch = ( + f"SELECT {inner_select_list} FROM {l_table_ident} a " + f"WHERE a.{q(l_chrom)} IS NOT NULL " + f"AND a.{q(l_chrom)} NOT IN (" + f"SELECT DISTINCT {q(r_chrom)} FROM {r_table_ident} " + f"WHERE {q(r_chrom)} IS NOT NULL)" + ) + + # The same result without the per-chromosome partition, selected at + # execution time when the chromosome count exceeds the ceiling. The + # chromosome equality moves from the partition filter into the join or + # correlation predicate; ANTI keeps the IS NOT NULL guard so both forms + # agree about NULL-chromosome rows. + chrom_equality = f"a.{q(l_chrom)} = b.{q(r_chrom)}" + if sides.is_left_only_join: + null_guard = "" + if sides.kind == "ANTI": + null_guard = f"a.{q(l_chrom)} IS NOT NULL AND " + unpartitioned_query = ( + f"SELECT {inner_select_list} FROM {l_table_ident} a " + f"WHERE {null_guard}{exists_keyword} (" + f"SELECT 1 FROM {r_table_ident} b " + f"WHERE {chrom_equality} AND {predicate_sql})" ) else: - chrom_partition_subquery = ( - f"SELECT DISTINCT {q(l_chrom)} AS chrom FROM {l_table_ident} " - f"INTERSECT SELECT DISTINCT {q(r_chrom)} AS chrom " - f"FROM {r_table_ident}" + unpartitioned_query = ( + f"SELECT {inner_select_list} FROM {l_table_ident} a " + f"JOIN {r_table_ident} b " + f"ON {chrom_equality} AND {predicate_sql}" ) - set_var_stmt = set_variable_statement( - var_name, per_chrom_sql_expr, chrom_partition_subquery, empty_schema + var_name, set_var_stmt = set_variable_statement( + per_chrom_sql_expr, + chrom_partition_subquery, + empty_schema, + unpartitioned_query, + tail_branch=tail_branch, ) # Constant wrapper alias — uniqueness is provided by the @@ -1397,7 +1884,7 @@ def _build_sql( outer_select = " ".join(outer_select_parts) - return set_var_stmt + ";\n" + outer_select + return set_var_stmt, outer_select def _resolve_projections( self, @@ -1450,9 +1937,7 @@ def _resolve_projections( # A projection with an out-of-scope column reference is a user error the # naive plan also rejects, so it is left to the dispatch below to raise. for sel in query.expressions: - target = sel.this if isinstance(sel, exp.Alias) else sel - while isinstance(target, exp.Paren): - target = target.this + target = _unwrap_projection_target(sel) if _projection_declines_to_naive( target, l_alias, r_alias, sides.is_left_only_join ): @@ -1486,7 +1971,7 @@ def allocate(tbl: str, col: str) -> str: ``__giql_p`` ↔ source-column mapping (e.g. ``assert projection_map[(a, start)] == "__giql_p3"``). """ - tbl = _normalize_alias(tbl) + tbl = _fold_identifier(tbl) key = (tbl, col) existing = projection_map.get(key) if existing is not None: @@ -1508,7 +1993,7 @@ def allocate(tbl: str, col: str) -> str: def render_aggregate(agg_node: exp.Expression, user_alias: str | None) -> None: """Validate aggregate-argument qualifiers and emit the outer projection.""" for col in agg_node.find_all(exp.Column): - col_table = _normalize_alias(col.table) if col.table else "" + col_table = _fold_identifier(col.table) if col.table else "" if col_table not in (l_alias, r_alias): raise _UnqualifiedProjectionError( "dialect='duckdb' requires qualified aggregate " @@ -1536,7 +2021,7 @@ def render_aggregate(agg_node: exp.Expression, user_alias: str | None) -> None: if modifier_node is None: continue for col in modifier_node.find_all(exp.Column): - col_table = _normalize_alias(col.table) if col.table else "" + col_table = _fold_identifier(col.table) if col.table else "" if col_table in (l_alias, r_alias): reject_right_side_if_left_only(col_table, col.sql()) allocate(col_table, col.name) @@ -1548,7 +2033,7 @@ def render_aggregate(agg_node: exp.Expression, user_alias: str | None) -> None: # cannot resolve it), matching the SELECT-list / modifier behavior. for residual in outer_where_residuals or []: for col in residual.find_all(exp.Column): - col_table = _normalize_alias(col.table) if col.table else "" + col_table = _fold_identifier(col.table) if col.table else "" if col_table in (l_alias, r_alias): reject_right_side_if_left_only(col_table, col.sql()) allocate(col_table, col.name) @@ -1556,16 +2041,12 @@ def render_aggregate(agg_node: exp.Expression, user_alias: str | None) -> None: # Pre-allocate inner aliases for columns referenced inside any # aggregate function in the SELECT list (we'll rewrite the # aggregate's argument to use the inner alias when rendering the - # outer projection below). Peel any ``exp.Paren`` wrapper first - # so a paren-wrapped aggregate like ``(SUM(a.score)) AS s`` is - # treated identically to its bare form. + # outer projection below). for sel in query.expressions: - target = sel.this if isinstance(sel, exp.Alias) else sel - while isinstance(target, exp.Paren): - target = target.this + target = _unwrap_projection_target(sel) if isinstance(target, exp.AggFunc): for col in target.find_all(exp.Column): - col_table = _normalize_alias(col.table) if col.table else "" + col_table = _fold_identifier(col.table) if col.table else "" if col_table in (l_alias, r_alias): reject_right_side_if_left_only(col_table, col.sql()) allocate(col_table, col.name) @@ -1573,13 +2054,8 @@ def render_aggregate(agg_node: exp.Expression, user_alias: str | None) -> None: outer_projections: list[str] = [] for sel in query.expressions: - target = sel.this if isinstance(sel, exp.Alias) else sel + target = _unwrap_projection_target(sel) user_alias = sel.alias if isinstance(sel, exp.Alias) else None - # Peel paren wrappers at the top of dispatch so a paren-wrapped - # aggregate like ``(SUM(a.score)) AS s`` routes through the AggFunc - # branch below rather than the generic catch-all. - while isinstance(target, exp.Paren): - target = target.this # Star projections (bare ``*`` and ``a.*`` / ``b.*``) never reach # here — :func:`_has_star_projection` declines them upstream (#202) — @@ -1592,7 +2068,7 @@ def render_aggregate(agg_node: exp.Expression, user_alias: str | None) -> None: continue if isinstance(target, exp.Column) and target.table: - tbl = _normalize_alias(target.table) + tbl = _fold_identifier(target.table) reject_right_side_if_left_only(tbl, target.sql()) col = target.name alias = allocate(tbl, col) @@ -1614,7 +2090,7 @@ def render_aggregate(agg_node: exp.Expression, user_alias: str | None) -> None: # than the generic "qualify the column" one, which would steer the # user toward another invalid form (``b.col`` is never valid here). for col in target.find_all(exp.Column): - col_table = _normalize_alias(col.table) if col.table else "" + col_table = _fold_identifier(col.table) if col.table else "" reject_right_side_if_left_only(col_table, target.sql()) # Otherwise the fault is an unqualified / unknown-table column; the # accurate guidance is to qualify it (once qualified, the wrapper @@ -1671,6 +2147,30 @@ def _has_sibling_spatial_predicate(node: Intersects, root: exp.Expression) -> bo return False +def _decline_to_generic(node: exp.Expression, ctx: ExpansionContext) -> exp.Expression: + """Expand *node* with whatever expander serves ``(GenericTarget, Intersects)``. + + Going through the registry keeps this target's fallback and the generic + target's expansion the same thing by construction. Calling the built-in + directly made them the same only by coincidence: a user override registered + on ``(GenericTarget, Intersects)`` took effect under ``dialect=None`` and was + silently ignored under ``dialect="duckdb"`` for every shape this override + declines. + + Only the generic slot is consulted. Resolving this target's own slot would + return this function's caller and recurse without bound, since + :meth:`~giql.expander.ExpanderRegistry.resolve` tries the exact + ``(target, operator)`` entry first. + + Falls back to the built-in when the generic slot is empty, which a cleared + registry produces. + """ + generic = REGISTRY.resolve(GenericTarget(), Intersects) + if generic is not None: + return generic(node, ctx) + return _expand_spatial_op(node, ctx, "intersects") + + @register(DuckDBTarget, Intersects) def expand_intersects_duckdb( node: exp.Expression, ctx: ExpansionContext @@ -1688,12 +2188,13 @@ def expand_intersects_duckdb( built SQL (which serializes verbatim), and returns the ``Intersects`` node unchanged (mirroring the CLUSTER / MERGE whole-query expanders). - Every other shape — a join the IEJoin path declines (LEFT/RIGHT/FULL, self-join, - multiple INTERSECTS, extra predicates, non-base operands, 3+ tables), a - literal-range predicate, or a residual column-to-column predicate — defers to - :func:`giql.expanders.intersects._expand_spatial_op`, the same naive overlap - predicate the ``(GenericTarget, Intersects)`` expander emits on every target - (#167). A query carrying any *sibling* spatial predicate also defers (see + Every other shape — a join the IEJoin path declines (FULL OUTER, the LEFT / + RIGHT shapes the decomposition declines, self-join, multiple INTERSECTS, + extra predicates, non-base operands, 3+ tables), a literal-range predicate, + or a residual column-to-column predicate — defers to whichever expander + serves ``(GenericTarget, Intersects)`` (see :func:`_decline_to_generic`), + which by default is the same naive overlap predicate every other target + emits (#167). A query carrying any *sibling* spatial predicate also defers (see :func:`_has_sibling_spatial_predicate`), so the IEJoin never inlines a residual it cannot see. ``transform_to_sql`` then runs on the pass-3 AST, which for the shapes the IEJoin path accepts is structurally identical to the raw @@ -1712,4 +2213,4 @@ def expand_intersects_duckdb( if iejoin_sql is not None: ctx.add_statement_finalizer(lambda _root: exp.Command(this=iejoin_sql)) return node - return _expand_spatial_op(node, ctx, "intersects") + return _decline_to_generic(node, ctx) diff --git a/src/giql/table.py b/src/giql/table.py index 563343c..a0a2068 100644 --- a/src/giql/table.py +++ b/src/giql/table.py @@ -13,7 +13,7 @@ from giql.constants import DEFAULT_STRAND_COL -@dataclass +@dataclass(frozen=True) class Table: """Genomic table configuration for transpilation. @@ -21,6 +21,14 @@ class Table: mapping a pseudo-column name (genomic_col) to the physical columns that store chromosome, start, end, and optionally strand information. + Instances are immutable. ``coordinate_system`` and ``interval_type`` select + the coordinate translation the emitted SQL performs, so a value outside the + documented pair would produce a predicate matching no coordinate system at + all. Validation therefore has to hold for the object's whole lifetime, not + only at construction -- freezing is what makes ``__post_init__`` the single + way in. Build a variant with :func:`dataclasses.replace` rather than by + assignment. Immutability also makes the value hashable and value-equal. + Parameters ---------- name : str diff --git a/src/giql/targets.py b/src/giql/targets.py index f5031aa..d487ce3 100644 --- a/src/giql/targets.py +++ b/src/giql/targets.py @@ -18,6 +18,7 @@ """ from dataclasses import dataclass +from typing import Literal @dataclass(frozen=True) @@ -180,19 +181,26 @@ class DataFusionTarget(Target): # Public dialect names only. ``generic`` is intentionally absent: ``None`` is # the sole public way to select it (see :func:`resolve_target`). +#: The dialect names ``transpile`` ships. Typing the parameter as +#: ``DialectName | str`` keeps editor completion for the built-ins while still +#: admitting a custom target's registered name; the union widens to ``str`` for +#: a type checker, which is the point -- the registry is open. +DialectName = Literal["duckdb", "datafusion"] + _TARGETS_BY_NAME: dict[str, type[Target]] = { DuckDBTarget.name: DuckDBTarget, DataFusionTarget.name: DataFusionTarget, } -def resolve_target(dialect: str | None) -> Target: +def resolve_target(dialect: DialectName | str | Target | None) -> Target: """Resolve a ``dialect`` parameter to a :class:`Target` instance. Parameters ---------- - dialect : str | None - The target dialect name. ``None`` resolves to :class:`GenericTarget`; + dialect : DialectName | str | Target | None + A :class:`Target` instance, which resolves to itself, or the name of + one. ``None`` resolves to :class:`GenericTarget`; ``"duckdb"`` and ``"datafusion"`` resolve to their respective built-in targets. Any other name is resolved against the plugin registry — a custom :class:`Target` declared through @@ -209,6 +217,12 @@ def resolve_target(dialect: str | None) -> Target: ValueError If *dialect* is neither a built-in name nor a registered custom target. """ + if isinstance(dialect, Target): + # Targets are frozen and value-equal, so handing one back is safe and + # spares the caller a round trip through the registry to select an + # object it already holds. + return dialect + if dialect is None: return GenericTarget() diff --git a/src/giql/transpile.py b/src/giql/transpile.py index 94e85f0..40b80ee 100644 --- a/src/giql/transpile.py +++ b/src/giql/transpile.py @@ -6,8 +6,6 @@ from contextlib import contextmanager from typing import Iterator -from typing import Literal -from typing import overload from sqlglot import parse_one @@ -18,45 +16,16 @@ from giql.resolver import resolve_operator_refs from giql.table import Table from giql.table import Tables +from giql.targets import DialectName +from giql.targets import Target from giql.targets import resolve_target -@overload def transpile( giql: str, tables: list[str | Table] | None = None, *, - dialect: Literal["datafusion"] | None = None, -) -> str: ... - - -@overload -def transpile( - giql: str, - tables: list[str | Table] | None = None, - *, - dialect: Literal["duckdb"], -) -> str: ... - - -# Widened overload for arbitrary custom-target dialect names (the registry -# plugin hub). It subsumes the ``Literal["duckdb"]`` overload above, which is -# kept deliberately so editors still autocomplete the ``"duckdb"`` literal -# (symmetric with the ``Literal["datafusion"]`` overload). -@overload -def transpile( - giql: str, - tables: list[str | Table] | None = None, - *, - dialect: str, -) -> str: ... - - -def transpile( - giql: str, - tables: list[str | Table] | None = None, - *, - dialect: str | None = None, + dialect: DialectName | str | Target | None = None, ) -> str: """Transpile a GIQL query to SQL. @@ -74,28 +43,45 @@ def transpile( Table configurations. Strings use default column mappings (chrom, start, end, strand). :class:`Table` objects provide custom column name mappings. - dialect : str | None - Optional target engine. Resolves to a :class:`giql.targets.Target` - carrying the engine's capability set; ``None`` selects the generic - portable target, ``"duckdb"`` / ``"datafusion"`` the built-in targets, - and any other name a custom :class:`giql.targets.Target` registered on - the plugin hub (see :func:`giql.expander.register` / - :meth:`giql.expander.ExpanderRegistry.register_target`). When set to - ``"duckdb"``, column-to-column - ``INTERSECTS`` joins (INNER, SEMI, or ANTI) are transpiled into a - per-chromosome dynamic-SQL pattern (``SET VARIABLE`` + - ``query(getvariable(...))``) that DuckDB plans through its - range-join family (``IE_JOIN`` / ``PIECEWISE_MERGE_JOIN``). Shapes - the IEJoin path declines (LEFT/RIGHT/FULL joins, self-joins, multiple - INTERSECTS, extra predicates, non-base operands, 3+ tables) fall - through to the generic naive overlap predicate. ``"datafusion"`` and + dialect : DialectName | str | Target | None + Optional target engine, as either a :class:`giql.targets.Target` or the + name of one. A Target carries the engine's capability set; ``None`` + selects the generic portable target, ``"duckdb"`` / ``"datafusion"`` the + built-in targets, and any other name a custom + :class:`giql.targets.Target` registered on the plugin hub (see + :func:`giql.expander.register` / + :meth:`giql.expander.ExpanderRegistry.register_target`). Passing the + instance directly is the way to use a one-off Target without registering + it under a name, since registration is process-global and outlives the + call. :class:`giql.targets.GenericTarget` is accepted as an instance but + has no selectable name -- ``None`` is the one public spelling for it. When set to + ``"duckdb"``, column-to-column ``INTERSECTS`` joins (INNER, SEMI, ANTI, + and LEFT / RIGHT outer) are transpiled into a per-chromosome dynamic-SQL + pattern (``SET VARIABLE`` + ``query(getvariable(...))``) that DuckDB + plans through its range-join family (``IE_JOIN`` / + ``PIECEWISE_MERGE_JOIN``). A LEFT or RIGHT outer join is decomposed into + an INNER half, a ``NOT EXISTS`` half carrying the preserved side's + unmatched rows, and a third branch carrying its NULL-chromosome rows, + combined with ``UNION ALL``; the first two reach the fast operator and + the third is a filtered scan. Because the per-chromosome form emits one + branch per distinct chromosome, it degrades to a single non-partitioned + query above a partition-count ceiling, decided at execution time: the + partitioned form wins by orders of magnitude on a primary assembly and + loses by orders of magnitude on a scaffold-level one. Shapes the IEJoin + path declines fall through to the generic naive overlap predicate; the + performance guide enumerates them, and that enumeration is deliberately + not repeated here, because a second copy drifts. ``"datafusion"`` and ``None`` always emit that naive predicate — a plain ``ON a.chrom = b.chrom AND a.start < b.end AND b.start < a.end`` condition the engine's own optimizer plans as a range join — for both - inner and outer column-to-column INTERSECTS joins. Hard-error - projection shapes raise ``ValueError`` at transpile time; see the - performance guide for the full enumeration. The target's capabilities - also choose the + inner and outer column-to-column INTERSECTS joins. For the INNER / + SEMI / ANTI shapes, a projection the rewrite cannot attribute to a side + raises ``ValueError`` at transpile time. The outer-join form instead + declines silently to the naive predicate for every projection it cannot + split, so ``dialect="duckdb"`` never changes an outer join's result — + only its plan. See the performance guide for the full enumeration. + + The target's capabilities also choose the coordinate-canonicalization emit form for a non-canonically-encoded table: ``"duckdb"`` emits ``SELECT * REPLACE (...)``, while the generic (``None``) and ``"datafusion"`` targets emit the portable @@ -108,13 +94,25 @@ def transpile( Returns ------- str - The transpiled SQL query. + The transpiled SQL. Every target but ``"duckdb"`` returns a single + statement, as does ``"duckdb"`` for any shape its IEJoin path declines. + An accelerated column-to-column ``INTERSECTS`` join instead returns a + multi-statement script: one ``SET VARIABLE`` statement, or two for a + decomposed outer join, followed by the final ``SELECT``. Execute it on a + single connection -- a driver that splits statements, or forwards only + the last, drops the variable the ``SELECT`` reads and yields empty or + NULL results. Variable names are derived from the content they hold, so + re-running one query shape rebinds its own names rather than leaving a + fresh pair behind per call. Raises ------ ValueError If the query cannot be parsed or transpiled, or if ``dialect`` is unknown. + TypeError + If an entry in ``tables`` is neither a ``str`` nor a + :class:`giql.table.Table`. Examples -------- @@ -149,8 +147,9 @@ def transpile( tables=["peaks", "genes"], ) - DuckDB IEJoin dialect (column-to-column INNER/SEMI/ANTI JOIN only; - projections must be qualified):: + DuckDB IEJoin dialect (column-to-column INNER/SEMI/ANTI/LEFT/RIGHT JOIN). + INNER, SEMI and ANTI require qualified projections and raise otherwise; the + outer-join forms decline to the naive predicate instead:: sql = transpile( "SELECT a.chrom, a.start, b.start " @@ -236,8 +235,16 @@ def _build_tables(tables: list[str | Table] | None) -> Tables: for item in tables: if isinstance(item, str): container.register(item, Table(item)) - else: + elif isinstance(item, Table): container.register(item.name, item) + else: + # Reject here rather than duck-typing ``item.name``. An object that + # merely looks like a Table reaches pass 1 and fails there with an + # AttributeError naming an internal column attribute, which tells a + # caller nothing about which argument was wrong. + raise TypeError( + f"tables entries must be str or Table, got {type(item).__name__}" + ) return container diff --git a/tests/integration/bedtools/test_cluster.py b/tests/integration/bedtools/test_cluster.py index d88c31b..7ae2f9e 100644 --- a/tests/integration/bedtools/test_cluster.py +++ b/tests/integration/bedtools/test_cluster.py @@ -6,12 +6,16 @@ number of distinct clusters should equal the number of merged intervals. """ +import pytest + from giql import transpile from .utils.bedtools_wrapper import merge from .utils.data_models import GenomicInterval from .utils.duckdb_loader import load_intervals +pytestmark = pytest.mark.integration + def test_cluster_basic(duckdb_connection): """ diff --git a/tests/integration/bedtools/test_contains.py b/tests/integration/bedtools/test_contains.py index 6325e43..87fe584 100644 --- a/tests/integration/bedtools/test_contains.py +++ b/tests/integration/bedtools/test_contains.py @@ -5,8 +5,12 @@ equivalent exists, so tests validate against known expected results. """ +import pytest + from .utils.data_models import GenomicInterval +pytestmark = pytest.mark.integration + def test_contains_point(giql_query): """ diff --git a/tests/integration/bedtools/test_distance.py b/tests/integration/bedtools/test_distance.py index 32f2be0..71ede19 100644 --- a/tests/integration/bedtools/test_distance.py +++ b/tests/integration/bedtools/test_distance.py @@ -10,6 +10,8 @@ from .utils.bedtools_wrapper import closest from .utils.data_models import GenomicInterval +pytestmark = pytest.mark.integration + def _oracle_distance(a: GenomicInterval, b: GenomicInterval) -> int: """Return the bedtools ``closest -d`` distance between a single A and B.""" diff --git a/tests/integration/bedtools/test_intersect.py b/tests/integration/bedtools/test_intersect.py index f4bfd43..c66ea3f 100644 --- a/tests/integration/bedtools/test_intersect.py +++ b/tests/integration/bedtools/test_intersect.py @@ -4,6 +4,8 @@ results to bedtools intersect command. """ +import pytest + from giql import transpile from .utils.bedtools_wrapper import intersect @@ -11,6 +13,8 @@ from .utils.data_models import GenomicInterval from .utils.duckdb_loader import load_intervals +pytestmark = pytest.mark.integration + def test_intersect_basic_overlap(duckdb_connection): """ @@ -325,3 +329,185 @@ def test_intersect_all_set_predicate(giql_query): names = {row[3] for row in result} assert names == {"large"}, f"Expected only large, got {names}" + + +def test_intersect_left_outer_join_duckdb_dialect(duckdb_connection): + """ + GIVEN interval sets where one A interval has no overlap on a shared + chromosome and another sits on a chromosome absent from B + WHEN a LEFT JOIN INTERSECTS query is transpiled with dialect="duckdb", + taking the INNER-plus-unmatched decomposition (#95) + THEN results match bedtools intersect -loj output exactly + + The dialect assertion matters as much as the comparison: this shape used to + decline to the naive-predicate plan, and the decomposition is only exercised + when the fast path actually fires. + """ + intervals_a = [ + GenomicInterval("chr1", 100, 200, "a1", 100, "+"), + GenomicInterval("chr1", 150, 250, "a2", 200, "+"), + GenomicInterval("chr1", 5000, 6000, "a_lonely", 300, "-"), + GenomicInterval("chrX", 10, 20, "a_other_chrom", 400, "+"), + ] + intervals_b = [ + GenomicInterval("chr1", 180, 220, "b1", 100, "+"), + GenomicInterval("chrY", 1, 5, "b_other_chrom", 200, "-"), + ] + + load_intervals( + duckdb_connection, + "intervals_a", + [i.to_tuple() for i in intervals_a], + ) + load_intervals( + duckdb_connection, + "intervals_b", + [i.to_tuple() for i in intervals_b], + ) + + bedtools_result = intersect( + [i.to_tuple() for i in intervals_a], + [i.to_tuple() for i in intervals_b], + loj=True, + ) + + sql = transpile( + """ + SELECT + a.chrom, a.start, a.end, a.name, a.score, a.strand, + b.chrom AS b_chrom, b.start AS b_start, b.end AS b_end, + b.name AS b_name, b.score AS b_score, b.strand AS b_strand + FROM intervals_a a + LEFT JOIN intervals_b b ON a.interval INTERSECTS b.interval + """, + tables=["intervals_a", "intervals_b"], + dialect="duckdb", + ) + assert sql.count("SET VARIABLE __giql_iejoin_") == 2 + giql_result = duckdb_connection.execute(sql).fetchall() + + comparison = compare_results(giql_result, bedtools_result) + assert comparison.match, comparison.failure_message() + + +def test_intersect_right_outer_join_duckdb_dialect(duckdb_connection): + """ + GIVEN interval sets where one B interval has no overlap on a shared + chromosome and another sits on a chromosome absent from A + WHEN a RIGHT JOIN INTERSECTS query is transpiled with dialect="duckdb", + taking the INNER-plus-unmatched decomposition (#95) + THEN results match bedtools intersect -loj with the inputs swapped + + bedtools has no -roj, but a RIGHT JOIN is -loj with A and B exchanged, so + the oracle is expressible by swapping both the inputs and the projection + order. This is the end-to-end check on the RIGHT path, which rewrites the + AST to swap the FROM and joined tables so one LEFT-shaped code path serves + both orientations. + """ + intervals_a = [ + GenomicInterval("chr1", 180, 220, "a1", 100, "+"), + GenomicInterval("chrY", 1, 5, "a_other_chrom", 200, "-"), + ] + intervals_b = [ + GenomicInterval("chr1", 100, 200, "b1", 100, "+"), + GenomicInterval("chr1", 150, 250, "b2", 200, "+"), + GenomicInterval("chr1", 5000, 6000, "b_lonely", 300, "-"), + GenomicInterval("chrX", 10, 20, "b_other_chrom", 400, "+"), + ] + + load_intervals( + duckdb_connection, + "intervals_a", + [i.to_tuple() for i in intervals_a], + ) + load_intervals( + duckdb_connection, + "intervals_b", + [i.to_tuple() for i in intervals_b], + ) + + bedtools_result = intersect( + [i.to_tuple() for i in intervals_b], + [i.to_tuple() for i in intervals_a], + loj=True, + ) + + sql = transpile( + """ + SELECT + b.chrom, b.start, b.end, b.name, b.score, b.strand, + a.chrom AS a_chrom, a.start AS a_start, a.end AS a_end, + a.name AS a_name, a.score AS a_score, a.strand AS a_strand + FROM intervals_a a + RIGHT JOIN intervals_b b ON a.interval INTERSECTS b.interval + """, + tables=["intervals_a", "intervals_b"], + dialect="duckdb", + ) + assert sql.count("SET VARIABLE __giql_iejoin_") == 2 + giql_result = duckdb_connection.execute(sql).fetchall() + + comparison = compare_results(giql_result, bedtools_result) + assert comparison.match, comparison.failure_message() + + +def test_intersect_left_outer_join_duckdb_dialect_with_duplicate_rows( + duckdb_connection, +): + """ + GIVEN byte-identical duplicate intervals on both sides, plus a duplicated + interval that matches nothing + WHEN a LEFT JOIN INTERSECTS query is transpiled with dialect="duckdb", + taking the INNER-plus-unmatched decomposition (#95) + THEN results match bedtools intersect -loj output exactly + + The other oracle cases draw from a unique-interval strategy, so duplicate + rows never reach this lane -- and row multiplicity is the axis a UNION ALL + rewrite is most likely to break, in either direction: a duplicate collapsed + by the matched half, or a row emitted by both halves at once. + """ + intervals_a = [ + GenomicInterval("chr1", 100, 200, "a1", 100, "+"), + GenomicInterval("chr1", 100, 200, "a1", 100, "+"), + GenomicInterval("chr1", 5000, 6000, "a_lonely", 300, "-"), + GenomicInterval("chr1", 5000, 6000, "a_lonely", 300, "-"), + ] + intervals_b = [ + GenomicInterval("chr1", 150, 250, "b1", 100, "+"), + GenomicInterval("chr1", 150, 250, "b1", 100, "+"), + ] + + load_intervals( + duckdb_connection, + "intervals_a", + [i.to_tuple() for i in intervals_a], + ) + load_intervals( + duckdb_connection, + "intervals_b", + [i.to_tuple() for i in intervals_b], + ) + + bedtools_result = intersect( + [i.to_tuple() for i in intervals_a], + [i.to_tuple() for i in intervals_b], + loj=True, + ) + + sql = transpile( + """ + SELECT + a.chrom, a.start, a.end, a.name, a.score, a.strand, + b.chrom AS b_chrom, b.start AS b_start, b.end AS b_end, + b.name AS b_name, b.score AS b_score, b.strand AS b_strand + FROM intervals_a a + LEFT JOIN intervals_b b ON a.interval INTERSECTS b.interval + """, + tables=["intervals_a", "intervals_b"], + dialect="duckdb", + ) + assert sql.count("SET VARIABLE __giql_iejoin_") == 2 + giql_result = duckdb_connection.execute(sql).fetchall() + + comparison = compare_results(giql_result, bedtools_result) + assert comparison.match, comparison.failure_message() diff --git a/tests/integration/bedtools/test_intersect_property.py b/tests/integration/bedtools/test_intersect_property.py index e6bd109..bb39d55 100644 --- a/tests/integration/bedtools/test_intersect_property.py +++ b/tests/integration/bedtools/test_intersect_property.py @@ -5,6 +5,7 @@ identical results to bedtools intersect. """ +import pytest from hypothesis import HealthCheck from hypothesis import given from hypothesis import settings @@ -17,7 +18,9 @@ from .utils.data_models import GenomicInterval from .utils.duckdb_loader import load_intervals -duckdb = __import__("pytest").importorskip("duckdb") +duckdb = pytest.importorskip("duckdb") + +pytestmark = pytest.mark.integration CHROMS = ["chr1", "chr2", "chr3"] @@ -215,6 +218,61 @@ def test_left_join_matches_bedtools_loj(intervals_a, intervals_b): assert comparison.match, comparison.failure_message() +@given( + intervals_a=unique_interval_list_st(max_size=30), + intervals_b=unique_interval_list_st(max_size=30), +) +@settings( + max_examples=40, + deadline=None, + suppress_health_check=[HealthCheck.too_slow], +) +def test_left_join_matches_bedtools_loj_on_duckdb_dialect(intervals_a, intervals_b): + """ + GIVEN two randomly generated sets of genomic intervals + WHEN GIQL LEFT JOIN INTERSECTS is transpiled with dialect="duckdb", taking + the INNER-plus-unmatched decomposition (#95) + THEN results match bedtools intersect -loj exactly + + This is a twin of ``test_left_join_matches_bedtools_loj`` rather than a + dialect parametrization of it: that query uses SELECT DISTINCT, which + declines the decomposition, so parametrizing would leave the fast path + unexercised while appearing to cover it. The projection here is + deliberately non-DISTINCT, which also keeps bedtools' record-oriented row + multiplicity intact. + """ + conn = duckdb.connect(":memory:") + try: + load_intervals(conn, "intervals_a", [i.to_tuple() for i in intervals_a]) + load_intervals(conn, "intervals_b", [i.to_tuple() for i in intervals_b]) + + sql = transpile( + """ + SELECT + a.chrom, a.start, a.end, a.name, a.score, a.strand, + b.chrom AS b_chrom, b.start AS b_start, b.end AS b_end, + b.name AS b_name, b.score AS b_score, b.strand AS b_strand + FROM intervals_a a + LEFT JOIN intervals_b b ON a.interval INTERSECTS b.interval + """, + tables=["intervals_a", "intervals_b"], + dialect="duckdb", + ) + assert sql.count("SET VARIABLE __giql_iejoin_") == 2 + giql_result = conn.execute(sql).fetchall() + finally: + conn.close() + + bedtools_result = intersect( + [i.to_tuple() for i in intervals_a], + [i.to_tuple() for i in intervals_b], + loj=True, + ) + + comparison = compare_results(giql_result, bedtools_result) + assert comparison.match, comparison.failure_message() + + @given( intervals_a=unique_interval_list_st(max_size=30), intervals_b=unique_interval_list_st(max_size=30), diff --git a/tests/integration/bedtools/test_merge.py b/tests/integration/bedtools/test_merge.py index 7783611..929b97d 100644 --- a/tests/integration/bedtools/test_merge.py +++ b/tests/integration/bedtools/test_merge.py @@ -4,6 +4,8 @@ results to bedtools merge command. """ +import pytest + from giql import transpile from .utils.bedtools_wrapper import merge @@ -11,6 +13,8 @@ from .utils.data_models import GenomicInterval from .utils.duckdb_loader import load_intervals +pytestmark = pytest.mark.integration + def test_merge_adjacent_intervals(duckdb_connection): """ diff --git a/tests/integration/bedtools/test_nearest.py b/tests/integration/bedtools/test_nearest.py index 4b0ea8d..5784cc6 100644 --- a/tests/integration/bedtools/test_nearest.py +++ b/tests/integration/bedtools/test_nearest.py @@ -4,12 +4,16 @@ consistent with bedtools closest command. """ +import pytest + from giql import transpile from .utils.bedtools_wrapper import closest from .utils.data_models import GenomicInterval from .utils.duckdb_loader import load_intervals +pytestmark = pytest.mark.integration + def test_nearest_non_overlapping(duckdb_connection): """ diff --git a/tests/integration/bedtools/test_nearest_property.py b/tests/integration/bedtools/test_nearest_property.py index 7c14a09..041d46c 100644 --- a/tests/integration/bedtools/test_nearest_property.py +++ b/tests/integration/bedtools/test_nearest_property.py @@ -21,6 +21,8 @@ duckdb = pytest.importorskip("duckdb") +pytestmark = pytest.mark.integration + def _expected_distance(ref: GenomicInterval, cand: GenomicInterval) -> int: """Compute the bedtools-parity distance between two half-open intervals. diff --git a/tests/integration/bedtools/test_strand_aware.py b/tests/integration/bedtools/test_strand_aware.py index f9c8eb9..80f2cca 100644 --- a/tests/integration/bedtools/test_strand_aware.py +++ b/tests/integration/bedtools/test_strand_aware.py @@ -4,6 +4,8 @@ operations, matching bedtools behavior with -s and -S flags. """ +import pytest + from giql import transpile from .utils.bedtools_wrapper import closest @@ -13,6 +15,8 @@ from .utils.data_models import GenomicInterval from .utils.duckdb_loader import load_intervals +pytestmark = pytest.mark.integration + def test_intersect_same_strand(duckdb_connection): """ diff --git a/tests/integration/bedtools/test_within.py b/tests/integration/bedtools/test_within.py index f2935b5..fcb6037 100644 --- a/tests/integration/bedtools/test_within.py +++ b/tests/integration/bedtools/test_within.py @@ -5,8 +5,12 @@ equivalent exists, so tests validate against known expected results. """ +import pytest + from .utils.data_models import GenomicInterval +pytestmark = pytest.mark.integration + def test_within_basic(giql_query): """ diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 6729a49..00a440b 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -105,7 +105,11 @@ def cross_target_oracle(): ``columns``. Returns the per-target normalized result so callers may make extra - assertions. + assertions, and records the per-target SQL on the fixture callable as + ``cross_target_oracle.sql_by_target`` so a case can additionally assert + which plan a target took. Row identity alone cannot: a target that stops + taking its intended fast path still returns the same rows, so a case + written to cover that path would keep passing while covering nothing. """ def _oracle( @@ -148,6 +152,7 @@ def _oracle( results[target] = ENGINE_RUNNERS[engine](sql, table_data, columns) assert_cross_target(results, expected_rows, sql_by_target) + _oracle.sql_by_target = sql_by_target return results return _oracle diff --git a/tests/integration/datafusion/test_cross_target_oracle.py b/tests/integration/datafusion/test_cross_target_oracle.py index 3eced51..70a702f 100644 --- a/tests/integration/datafusion/test_cross_target_oracle.py +++ b/tests/integration/datafusion/test_cross_target_oracle.py @@ -133,16 +133,19 @@ def test_column_to_column_left_join_preserves_unmatched(self, cross_target_oracl Peaks and genes where one peak overlaps a gene on chr1, one peak on chr1 has no overlap, and one peak sits on chr2 with no overlapping gene. When: - A column-to-column ``LEFT JOIN ... ON ... INTERSECTS`` runs. The DuckDB - IEJoin path declines outer joins, so every target — generic, datafusion, - and duckdb — evaluates the naive overlap predicate as a plain outer - ``ON`` condition. + A column-to-column ``LEFT JOIN ... ON ... INTERSECTS`` runs. Generic + and datafusion evaluate the naive overlap predicate as a plain outer + ``ON`` condition; duckdb decomposes the join into an INNER half plus + an unmatched half unioned together (#95). The targets reach the + answer by different plans, which is exactly what makes agreement + between them worth asserting. Then: Every target should return the matched pair plus both unmatched left - rows with a NULL right column, proving the naive predicate gives correct - outer-join semantics identically on every engine (#167). + rows with a NULL right column, proving all three targets agree on + outer-join semantics despite reaching them by different plans + (#167, #95). """ - # Arrange / Act / Assert + # Arrange / Act cross_target_oracle( "SELECT a.chrom, a.start AS a_start, b.start AS b_start " "FROM peaks a LEFT JOIN genes b ON a.interval INTERSECTS b.interval", @@ -163,6 +166,12 @@ def test_column_to_column_left_join_preserves_unmatched(self, cross_target_oracl ], ) + # Assert + # Without this the case would pass unchanged if duckdb stopped + # decomposing -- the naive plan returns these same rows. + duckdb_sql = cross_target_oracle.sql_by_target["duckdb"] + assert duckdb_sql.count("SET VARIABLE __giql_iejoin_") == 2 + class TestCrossTargetOracleContainsWithin: """CONTAINS and WITHIN identity across all targets.""" diff --git a/tests/test_duckdb_iejoin.py b/tests/test_duckdb_iejoin.py index f09f736..4a4f9a4 100644 --- a/tests/test_duckdb_iejoin.py +++ b/tests/test_duckdb_iejoin.py @@ -1,11 +1,17 @@ """Tests for the DuckDB IEJoin dialect path on column-to-column INTERSECTS joins.""" import re +import time import pytest +from sqlglot import parse_one from giql import Table from giql import transpile +from giql.dialect import GIQLDialect +from giql.expanders._per_chrom import MAX_PER_CHROM_PARTITIONS +from giql.expanders.intersects_duckdb import IntersectsDuckDBIEJoinTransformer +from giql.table import Tables duckdb = pytest.importorskip("duckdb") hypothesis = pytest.importorskip("hypothesis") @@ -81,19 +87,105 @@ def _python_count_overlaps(peaks: list[tuple], genes: list[tuple]) -> list[tuple return sorted(out) -def _explain_dynamic_sql(conn, sql: str) -> str: +def _make_interval_tables(conn, peak_rows: list[tuple], gene_rows: list[tuple]) -> None: + """Recreate bare (chrom, start, end) peaks/genes tables from row triples.""" + for table, rows in (("peaks", peak_rows), ("genes", gene_rows)): + conn.execute(f"DROP TABLE IF EXISTS {table}") + conn.execute( + f'CREATE TABLE {table} (chrom VARCHAR, "start" INTEGER, "end" INTEGER)' + ) + if rows: + conn.executemany(f"INSERT INTO {table} VALUES (?, ?, ?)", rows) + + +def _null_key(row: tuple) -> tuple: + """Total sort key for rows that may contain NULLs, which are not comparable.""" + return tuple((value is None, str(value)) for value in row) + + +def _python_left_join_overlap(peaks: list[tuple], genes: list[tuple]) -> list[tuple]: + """Reference LEFT-join overlap: every peak, paired with its genes or NULLs. + + Returns a *sorted multiset* (list) so callers can assert exact row + multiplicity. That matters more here than for the other references: the + dialect emits this shape as a UNION ALL of two halves, and the most + plausible way that rewrite breaks is emitting a row twice or not at all -- + which a `set` comparison would hide entirely. + """ + out: list[tuple] = [] + for pc, ps, pe in peaks: + # ``pc is not None`` encodes SQL NULL semantics: the per-chromosome + # partition is an equality, and NULL never equals anything -- not even + # another NULL -- so a NULL-chrom interval is always unmatched. + matches = [ + (gc, gs, ge) + for gc, gs, ge in genes + if pc is not None and pc == gc and pe > gs and ge > ps + ] + if matches: + out.extend((pc, ps, pe, gc, gs, ge) for gc, gs, ge in matches) + else: + out.append((pc, ps, pe, None, None, None)) + return sorted(out, key=_null_key) + + +def _rows_and_schema(conn, sql: str) -> tuple[list, list]: + """Execute *sql* and return its rows (NULL-safe sorted) with its result schema.""" + cursor = conn.execute(sql) + rows = sorted(cursor.fetchall(), key=_null_key) + return rows, [(d[0], d[1]) for d in cursor.description] + + +def _assert_agrees_with_naive(conn, query: str, tables=("peaks", "genes")) -> str: + """Assert the duckdb dialect matches the naive plan in rows AND result schema. + + Returns the dialect SQL so callers can additionally assert which path ran. + The schema matters as much as the rows: a UNION ALL branch that binds the + wrong column silently widens an INTEGER column to VARCHAR, and a derived + table wrapped around a branch silently renames duplicate output columns + (``chrom`` to ``chrom_1``) -- neither of which a row-only comparison of + stringified values can see. + """ + dialect_sql = transpile(query, tables=list(tables), dialect="duckdb") + naive_sql = transpile(query, tables=list(tables), dialect=None) + assert _rows_and_schema(conn, dialect_sql) == _rows_and_schema(conn, naive_sql) + return dialect_sql + + +def _timed_rows(conn, sql: str) -> tuple[float, list[tuple]]: + """Return the wall-clock seconds and rows from executing *sql* on *conn*.""" + started = time.perf_counter() + rows = conn.execute(sql).fetchall() + return time.perf_counter() - started, rows + + +def _dynamic_sql_variables(sql: str) -> list[str]: + """Return every session-variable name in *sql*, in emission order. + + A decomposed outer join declares two (the matched half then the unmatched + half); every other IEJoin shape declares one. Callers must say which they + mean -- matching only the first would silently plan the matched half and + report nothing about the other. + """ + return list(dict.fromkeys(re.findall(r"__giql_iejoin_[0-9a-f]+", sql))) + + +def _explain_dynamic_sql(conn, sql: str, half: int = 0) -> str: """Return the DuckDB EXPLAIN text of the per-chromosome dynamic SQL for *sql*. - Runs the leading ``SET VARIABLE`` statement, reads the aggregated - per-chromosome ``UNION ALL`` string back out of the session variable, and - returns the plain EXPLAIN for it. The outer ``query(getvariable(...))`` - wrapper is opaque to EXPLAIN, so the dynamic SQL must be planned directly. + Runs the leading ``SET VARIABLE`` statements, reads the aggregated + per-chromosome ``UNION ALL`` string back out of the session variable named by + *half*, and returns the plain EXPLAIN for it. The outer + ``query(getvariable(...))`` wrapper is opaque to EXPLAIN, so the dynamic SQL + must be planned directly. *half* selects which variable to plan: ``0`` is the + only one for a single-relation shape and the matched half of a decomposed + outer join, ``1`` is that decomposition's unmatched half. """ statements = [s for s in sql.split(";\n") if s.strip()] for statement in statements[:-1]: conn.execute(statement) - var_name = re.search(r"__giql_iejoin_[0-9a-f]+", sql).group(0) - dynamic_sql = conn.execute(f"SELECT getvariable('{var_name}')").fetchone()[0] + var_names = _dynamic_sql_variables(sql) + dynamic_sql = conn.execute(f"SELECT getvariable('{var_names[half]}')").fetchone()[0] return conn.execute("EXPLAIN " + dynamic_sql).fetchone()[1] @@ -170,10 +262,10 @@ def test_transpile_should_keep_naive_predicate_path_when_dialect_is_none(self): assert "SET VARIABLE" not in sql.upper() assert "getvariable" not in sql - def test_transpile_should_route_outer_join_intersects_to_naive_predicate_plan_when_dialect_is_duckdb( + def test_transpile_should_preserve_left_join_semantics_when_dialect_is_duckdb( self, conn ): - """Test that LEFT JOIN INTERSECTS falls back to the naive-predicate plan. + """Test that LEFT JOIN INTERSECTS keeps its semantics on the dialect path. Given: A LEFT JOIN INTERSECTS query and a peak with no overlapping @@ -181,9 +273,10 @@ def test_transpile_should_route_outer_join_intersects_to_naive_predicate_plan_wh When: The query is transpiled with ``dialect='duckdb'`` and executed. Then: - It should preserve LEFT JOIN semantics by returning the - unmatched peak with NULL gene columns (which the IEJoin path - cannot do, so the naive-predicate fallback fires). + It should take the IEJoin path and still return the unmatched peak + with NULL gene columns, via the INNER-plus-unmatched decomposition + (#95) rather than the naive-predicate fallback this shape used to + require. """ # Arrange _make_table( @@ -209,6 +302,8 @@ def test_transpile_should_route_outer_join_intersects_to_naive_predicate_plan_wh rows = sorted(conn.execute(sql).fetchall(), key=lambda r: r[0]) # Assert + assert sql.count("SET VARIABLE __giql_iejoin_") == 2 + assert "__giql_unmatched" in sql assert rows == [(100, 150), (5000, None)] def test_query_should_honor_extra_join_on_predicate_alongside_where_intersects_when_dialect_is_duckdb( @@ -503,6 +598,7 @@ def test_transpile_should_route_to_naive_predicate_plan_when_extra_predicate_use # Assert assert "SET VARIABLE __giql_iejoin_" not in sql + assert 'a."chrom" = b."chrom"' in sql def test_transpile_should_raise_when_extra_predicate_is_unqualified( self, @@ -1586,6 +1682,7 @@ def test_transpile_should_route_to_naive_predicate_plan_when_outer_join_intersec # Assert assert "SET VARIABLE __giql_iejoin_" not in sql + assert 'a."chrom" = b."chrom"' in sql def test_transpile_should_route_to_naive_predicate_plan_when_full_outer_join_intersects_lives_in_where( self, @@ -1612,6 +1709,7 @@ def test_transpile_should_route_to_naive_predicate_plan_when_full_outer_join_int # Assert assert "SET VARIABLE __giql_iejoin_" not in sql + assert 'a."chrom" = b."chrom"' in sql def test_transpile_should_route_to_naive_predicate_plan_when_extra_predicate_contains_window_function( self, @@ -1641,6 +1739,7 @@ def test_transpile_should_route_to_naive_predicate_plan_when_extra_predicate_con # Assert assert "SET VARIABLE __giql_iejoin_" not in sql + assert 'a."chrom" = b."chrom"' in sql def test_transpile_should_decline_window_aggregate_to_naive_plan(self): """Test that a window-aggregate projection declines to the naive plan. @@ -1753,6 +1852,7 @@ def test_transpile_should_route_to_naive_predicate_plan_when_modifier_has_subque # Assert assert "SET VARIABLE __giql_iejoin_" not in sql + assert 'a."chrom" = b."chrom"' in sql def test_transpile_should_route_to_naive_predicate_plan_when_modifier_has_exists( self, @@ -1782,6 +1882,7 @@ def test_transpile_should_route_to_naive_predicate_plan_when_modifier_has_exists # Assert assert "SET VARIABLE __giql_iejoin_" not in sql + assert 'a."chrom" = b."chrom"' in sql def test_transpile_should_route_to_naive_predicate_plan_when_having_has_subquery( self, @@ -1808,6 +1909,7 @@ def test_transpile_should_route_to_naive_predicate_plan_when_having_has_subquery # Assert assert "SET VARIABLE __giql_iejoin_" not in sql + assert 'a."chrom" = b."chrom"' in sql def test_transpile_should_accept_paren_wrapped_aggregate_in_select( self, @@ -1897,6 +1999,7 @@ def test_transpile_should_route_to_naive_predicate_plan_when_query_has_distinct_ # Assert assert "SET VARIABLE __giql_iejoin_" not in sql + assert 'a."chrom" = b."chrom"' in sql def test_transpile_should_route_to_naive_predicate_plan_and_execute_correctly_when_query_has_top_level_with_clause( self, conn @@ -1939,6 +2042,7 @@ def test_transpile_should_route_to_naive_predicate_plan_and_execute_correctly_wh # Assert assert "SET VARIABLE __giql_iejoin_" not in sql + assert 'a."chrom" = b."chrom"' in sql assert rows == [(100,)] def test_transpile_should_route_to_naive_predicate_plan_when_three_table_cross_join( @@ -1988,6 +2092,7 @@ def test_transpile_should_route_to_naive_predicate_plan_when_three_table_cross_j # Assert assert "SET VARIABLE __giql_iejoin_" not in sql + assert 'a."chrom" = b."chrom"' in sql # One (peaks, genes) overlap pair cross-joined with 2 extra rows # = 2 result rows. The dialect path would emit only 1 (extra dropped). assert len(rows) == 2 @@ -2020,6 +2125,7 @@ def test_transpile_should_route_to_naive_predicate_plan_when_target_is_giql_oper # Assert assert "SET VARIABLE __giql_iejoin_" not in sql + assert 'a."chrom" = b."chrom"' in sql # No empty-name table interpolation leaked into the output. assert "FROM " not in sql assert "FROM (SELECT * FROM " not in sql @@ -2228,6 +2334,44 @@ def test_transpile_should_decline_star_with_unknown_qualifier_to_naive_plan( assert "SET VARIABLE" not in sql.upper() assert "getvariable" not in sql + def test_transpile_should_reuse_variable_names_for_an_identical_query(self): + """Test that re-transpiling one query mints the same variable names. + + Given: + The same decomposed outer join transpiled twice. + When: + The session-variable names are compared. + Then: + They should be identical, and there should still be two distinct + names within one output. + + The names are content-addressed: each is a digest of everything that + determines its variable's value. Session variables live for the + connection and the emitted script cannot release them -- the final + statement has to be the ``SELECT`` for DuckDB to return a result -- so a + fresh name per call made a long-lived connection accumulate one entry per + query executed. Addressing by content bounds the set by the number of + distinct query shapes instead, since re-running a shape rebinds its own + name to a byte-identical value. + """ + # Arrange + query = ( + "SELECT a.chrom, a.start FROM peaks a " + "LEFT JOIN genes b ON a.interval INTERSECTS b.interval" + ) + + # Act + first = _dynamic_sql_variables( + transpile(query, tables=["peaks", "genes"], dialect="duckdb") + ) + second = _dynamic_sql_variables( + transpile(query, tables=["peaks", "genes"], dialect="duckdb") + ) + + # Assert + assert first == second + assert len(set(first)) == 2 + def test_transpile_should_use_unique_variable_names_across_interleaved_calls( self, conn ): @@ -2480,6 +2624,7 @@ def test_transpile_should_route_to_naive_predicate_plan_when_paren_wraps_interse # Assert assert "SET VARIABLE __giql_iejoin_" not in sql + assert 'a."chrom" = b."chrom"' in sql def test_transpile_should_route_to_naive_predicate_plan_when_residual_has_subquery( self, @@ -2507,6 +2652,7 @@ def test_transpile_should_route_to_naive_predicate_plan_when_residual_has_subque # Assert assert "SET VARIABLE __giql_iejoin_" not in sql + assert 'a."chrom" = b."chrom"' in sql def test_transpile_should_rewrite_group_by_multiple_columns_to_inner_aliases_when_dialect_is_duckdb( self, @@ -2675,10 +2821,10 @@ def test_transpile_should_emit_aggregate_of_expression_with_rewritten_inner_alia assert "a.start" not in outer_select assert "a.end" not in outer_select - def test_transpile_should_route_right_outer_join_intersects_to_naive_predicate_plan_when_dialect_is_duckdb( + def test_transpile_should_decompose_right_outer_join_intersects_when_dialect_is_duckdb( self, ): - """Test that ``RIGHT JOIN INTERSECTS`` falls back to the naive-predicate plan. + """Test that ``RIGHT JOIN INTERSECTS`` takes the decomposition path. Given: A query with a ``RIGHT JOIN`` whose ON contains a @@ -2686,8 +2832,9 @@ def test_transpile_should_route_right_outer_join_intersects_to_naive_predicate_p When: ``transpile`` is called with ``dialect='duckdb'``. Then: - ``_has_outer_join_intersects`` should detect the - outer-join side and route to the naive-predicate plan. + It should emit the two-half decomposition (#95), which mirrors the + LEFT form by swapping the FROM and joined tables, rather than + declining to the naive-predicate plan. """ # Arrange & act sql = transpile( @@ -2698,7 +2845,14 @@ def test_transpile_should_route_right_outer_join_intersects_to_naive_predicate_p ) # Assert - assert "SET VARIABLE __giql_iejoin_" not in sql + assert sql.count("SET VARIABLE __giql_iejoin_") == 2 + assert "__giql_unmatched" in sql + # The swap is the whole point: the unmatched half must carry the + # PRESERVED (right) side's left-only chromosomes in its tail branch. + # Asserting only on the variable count passes even if the swap never + # happens, so pin which table the tail branch preserves (#95). + assert 'FROM genes a WHERE a."chrom" IS NOT NULL' in sql + assert 'FROM peaks a WHERE a."chrom" IS NOT NULL' not in sql def test_transpile_should_route_to_naive_predicate_plan_when_join_target_is_subquery( self, @@ -2726,6 +2880,7 @@ def test_transpile_should_route_to_naive_predicate_plan_when_join_target_is_subq # Assert assert "SET VARIABLE __giql_iejoin_" not in sql + assert 'a."chrom" = b."chrom"' in sql def test_transpile_should_decline_literal_projection_to_naive_plan(self): """Test that a literal-only SELECT-list entry declines to the naive plan. @@ -2837,6 +2992,7 @@ def test_transpile_should_fall_back_to_naive_predicate_plan_when_join_is_natural # Assert assert "SET VARIABLE __giql_iejoin_" not in sql + assert 'a."chrom" = b."chrom"' in sql def test_transpile_should_engage_dialect_when_using_join_targets_chrom_only(self): """Test that ``USING(chrom)`` engages the dialect path. @@ -2885,6 +3041,7 @@ def test_transpile_should_fall_back_when_using_join_targets_non_chrom_column(sel # Assert assert "SET VARIABLE __giql_iejoin_" not in sql + assert 'a."chrom" = b."chrom"' in sql def test_transpile_should_fall_back_when_using_join_targets_multiple_columns(self): """Test that multi-column ``USING(chrom, strand)`` falls back. @@ -2909,6 +3066,7 @@ def test_transpile_should_fall_back_when_using_join_targets_multiple_columns(sel # Assert assert "SET VARIABLE __giql_iejoin_" not in sql + assert 'a."chrom" = b."chrom"' in sql def test_transpile_should_engage_dialect_when_join_is_implicit_cross_join(self): """Test that CROSS JOIN + WHERE INTERSECTS continues to engage the dialect. @@ -2962,6 +3120,7 @@ def test_transpile_should_fall_back_when_aggregate_argument_contains_subquery( # Assert assert "SET VARIABLE __giql_iejoin_" not in sql + assert 'a."chrom" = b."chrom"' in sql def test_transpile_should_fall_back_when_select_aggregate_contains_correlated_subquery( self, @@ -2989,6 +3148,7 @@ def test_transpile_should_fall_back_when_select_aggregate_contains_correlated_su # Assert assert "SET VARIABLE __giql_iejoin_" not in sql + assert 'a."chrom" = b."chrom"' in sql def test_transpile_should_resolve_projections_when_aliases_differ_in_case( self, @@ -3167,17 +3327,20 @@ def test_transpile_should_engage_dialect_for_anti_join_with_intersects(self): # Assert assert "SET VARIABLE __giql_iejoin_" in sql - def test_transpile_should_use_left_only_chrom_partition_when_join_is_anti(self): - """Test that ANTI JOIN's partition source is left-distinct only. + def test_transpile_should_use_shared_chrom_partition_when_join_is_anti(self): + """Test that ANTI JOIN partitions on shared chromosomes plus a tail branch. Given: An ANTI JOIN + INTERSECTS query. When: ``transpile`` is called with ``dialect='duckdb'``. Then: - The emitted SET VARIABLE block should NOT contain - ``INTERSECT`` (the partition source is left-distinct only, - not the chromosome-INTERSECT used by INNER / SEMI). + The emitted SET VARIABLE block should partition on the + chromosome ``INTERSECT`` like INNER / SEMI, and carry the left-only + chromosomes in a single non-partitioned tail branch rather than one + per-chromosome branch apiece (#95). A left-only chromosome can never + match, so a branch for it scans both tables to prove an emptiness the + partition already knows. """ # Arrange query = ( @@ -3190,7 +3353,8 @@ def test_transpile_should_use_left_only_chrom_partition_when_join_is_anti(self): set_var_stmt = sql.split(";\n", 1)[0] # Assert - assert "INTERSECT" not in set_var_stmt + assert "INTERSECT" in set_var_stmt + assert 'a."chrom" NOT IN (SELECT DISTINCT "chrom" FROM genes' in set_var_stmt def test_transpile_should_reject_right_side_projection_when_join_is_anti(self): """Test that ANTI JOIN raises on ``b.col`` in the outer SELECT. @@ -3247,7 +3411,10 @@ def test_transpile_should_emit_zero_fill_wrapper_when_left_join_count(self): assert "SET VARIABLE __giql_iejoin_" in sql assert "__giql_counts" in sql assert "COALESCE" in sql - assert 'a."chrom" = b."chrom"' not in sql + # The chromosome equality is legitimate inside the guarded + # non-partitioned fallback (#95); what must not happen is the + # per-chromosome template emitting it instead of partitioning on it. + assert 'a."chrom" = b."chrom"' not in sql.split("ELSE", 1)[1] def test_transpile_should_decline_count_overlaps_when_count_star(self): """Test that COUNT(*) over a LEFT join declines to the naive plan. @@ -3603,6 +3770,1472 @@ def test_count_overlaps_plan_should_avoid_nested_loop_when_executed_on_duckdb( assert "BLOCKWISE_NL_JOIN" not in plan +class TestTranspileDuckDBIEJoinOuterJoinDecomposition: + """Outer-join INTERSECTS as INNER pairs UNION ALL unmatched rows (#95).""" + + def test_transpile_should_emit_union_all_decomposition_when_left_join_intersects( + self, + ): + """Test that a LEFT-join INTERSECTS emits the two-half decomposition. + + Given: + A ``SELECT a.cols, b.cols FROM a LEFT JOIN b ON a.interval + INTERSECTS b.interval`` query. + When: + ``transpile`` is called with ``dialect='duckdb'``. + Then: + It should emit two ``SET VARIABLE`` blocks (the INNER half and the + unmatched ANTI half) as three ``UNION ALL`` branches, never the + naive ``a.chrom = b.chrom`` hash-join predicate. + """ + # Arrange + query = ( + 'SELECT a.chrom, a.start, a."end", b.chrom AS b_chrom ' + "FROM peaks a LEFT JOIN genes b ON a.interval INTERSECTS b.interval" + ) + + # Act + sql = transpile(query, tables=["peaks", "genes"], dialect="duckdb") + + # Assert + assert sql.count("SET VARIABLE __giql_iejoin_") == 2 + assert "NOT EXISTS" in sql + # Positional, not membership: a bare "UNION ALL" proves nothing here + # because it is also the string_agg separator inside every per-chrom + # dynamic-SQL template (see _per_chrom.set_variable_statement). Both + # seams are asserted, so all three branches are pinned. + assert re.search(r"AS __giql_iejoin_wrapper UNION ALL SELECT ", sql) + assert re.search(r"\) AS __giql_unmatched UNION ALL SELECT ", sql) + # The matched half is a bare SELECT, not a derived table: a + # ``SELECT * FROM (...)`` wrapper would rename duplicate output columns + # and that renaming would become the union's schema. + assert "__giql_matched" not in sql + assert 'NULL AS "b_chrom"' in sql + + def test_query_should_match_reference_left_join_when_dialect_is_duckdb(self, conn): + """Test that the decomposition reproduces LEFT JOIN semantics exactly. + + Given: + Peaks that overlap a gene, a peak with no overlap on a shared + chromosome, and a peak on a chromosome absent from the right table. + When: + The LEFT-join INTERSECTS query is transpiled with + ``dialect='duckdb'`` and executed. + Then: + It should return every peak — matched peaks paired with each + overlapping gene, unmatched peaks with NULL gene columns. + """ + # Arrange + peak_rows = [ + ("chr1", 100, 200), + ("chr1", 5000, 6000), + ("chrX", 10, 20), + ] + gene_rows = [("chr1", 150, 250), ("chr1", 180, 300), ("chrY", 1, 2)] + _make_interval_tables(conn, peak_rows, gene_rows) + sql = transpile( + 'SELECT a.chrom, a.start, a."end", b.chrom AS b_chrom, ' + 'b.start AS b_start, b."end" AS b_end ' + "FROM peaks a LEFT JOIN genes b ON a.interval INTERSECTS b.interval", + tables=["peaks", "genes"], + dialect="duckdb", + ) + + # Act + rows = sorted(conn.execute(sql).fetchall(), key=_null_key) + + # Assert + # The path assertion is load-bearing: the naive plan returns these + # same rows, so without it this test passes even if the + # decomposition silently stops firing. + assert sql.count("SET VARIABLE __giql_iejoin_") == 2 + assert rows == _python_left_join_overlap(peak_rows, gene_rows) + + def test_query_should_preserve_row_multiplicity_when_both_sides_have_duplicates( + self, conn + ): + """Test that duplicate rows on both sides keep their exact multiplicity. + + Given: + A peak duplicated on the left and a gene duplicated on the right, + so the matched half must emit the full cross product of duplicates. + When: + The LEFT-join INTERSECTS query is transpiled with + ``dialect='duckdb'`` and executed. + Then: + It should return the same multiset as the Python reference, with no + row collapsed or double-counted by the UNION ALL rewrite. + """ + # Arrange + peak_rows = [("chr1", 100, 200), ("chr1", 100, 200), ("chr1", 900, 950)] + gene_rows = [("chr1", 150, 250), ("chr1", 150, 250)] + _make_interval_tables(conn, peak_rows, gene_rows) + sql = transpile( + 'SELECT a.chrom, a.start, a."end", b.chrom AS b_chrom, ' + 'b.start AS b_start, b."end" AS b_end ' + "FROM peaks a LEFT JOIN genes b ON a.interval INTERSECTS b.interval", + tables=["peaks", "genes"], + dialect="duckdb", + ) + + # Act + rows = sorted(conn.execute(sql).fetchall(), key=_null_key) + + # Assert + # The path assertion is load-bearing: the naive plan returns these + # same rows, so without it this test passes even if the + # decomposition silently stops firing. + assert sql.count("SET VARIABLE __giql_iejoin_") == 2 + assert rows == _python_left_join_overlap(peak_rows, gene_rows) + + def test_query_should_resolve_shadowed_name_to_relation_column(self, conn): + """Test that a NULL-fill alias never shadows the preserved column it names. + + Given: + A projection whose NULL-side position is aliased to the same output + name as a preserved-side position and sits ahead of it, so the + unmatched half emits ``SELECT NULL AS "dup", "dup" AS "dup"`` -- a + bare column reference following a same-named computed alias in one + SELECT list. + When: + The LEFT-join INTERSECTS query is transpiled with + ``dialect='duckdb'`` and executed. + Then: + The bare reference should resolve to the subquery's relation column + and not to the lateral NULL alias declared just before it, so the + preserved value survives instead of being nulled out. + + The duplicate-name gate is deliberately preserved-side only, which + leaves this collision reachable; nothing else pins which way DuckDB + resolves it. + """ + # Arrange + _make_interval_tables( + conn, [("chr1", 100, 200), ("chr1", 5000, 6000)], [("chr1", 150, 250)] + ) + query = ( + "SELECT b.chrom AS dup, a.start AS dup FROM peaks a " + "LEFT JOIN genes b ON a.interval INTERSECTS b.interval" + ) + + # Act & assert + sql = _assert_agrees_with_naive(conn, query) + + # Assert + assert sql.count("SET VARIABLE __giql_iejoin_") == 2 + assert 'SELECT NULL AS "dup", "dup" AS "dup"' in sql + # The unmatched peak keeps its own start rather than inheriting the NULL. + assert (None, 5000) in conn.execute(sql).fetchall() + + def test_query_should_keep_duplicate_output_names_when_dialect_is_duckdb(self, conn): + """Test that the canonical bedtools projection keeps its column labels. + + Given: + The ``-wa -wb`` projection every bedtools migration writes, whose + six columns carry only three distinct output names (``chrom``, + ``start``, ``end`` from each side). + When: + The LEFT-join INTERSECTS query is transpiled with + ``dialect='duckdb'`` and executed. + Then: + It should return the naive plan's rows under the naive plan's + column names -- a performance flag must not renumber a user's + duplicate labels into ``chrom_1`` / ``start_1`` / ``end_1``. + """ + # Arrange + _make_interval_tables( + conn, [("chr1", 100, 200), ("chr1", 5000, 6000)], [("chr1", 150, 250)] + ) + query = ( + 'SELECT a.chrom, a.start, a."end", b.chrom, b.start, b."end" ' + "FROM peaks a LEFT JOIN genes b ON a.interval INTERSECTS b.interval" + ) + + # Act & assert + sql = _assert_agrees_with_naive(conn, query) + + # Assert + assert sql.count("SET VARIABLE __giql_iejoin_") == 2 + assert [d[0] for d in conn.execute(sql).description] == [ + "chrom", + "start", + "end", + "chrom", + "start", + "end", + ] + + def test_query_should_match_reference_right_join_when_dialect_is_duckdb(self, conn): + """Test that a RIGHT join preserves the right side's unmatched rows. + + Given: + A gene with no overlapping peak, plus a gene on a chromosome the + left table does not contain. + When: + The RIGHT-join INTERSECTS query is transpiled with + ``dialect='duckdb'`` and executed. + Then: + It should return every gene, matched ones paired with their peaks + and unmatched ones with NULL peak columns — the mirror of LEFT. + """ + # Arrange + peak_rows = [("chr1", 100, 200)] + gene_rows = [("chr1", 150, 250), ("chr1", 8000, 8100), ("chrY", 1, 5)] + _make_interval_tables(conn, peak_rows, gene_rows) + sql = transpile( + 'SELECT b.chrom, b.start, b."end", a.chrom AS a_chrom, ' + 'a.start AS a_start, a."end" AS a_end ' + "FROM peaks a RIGHT JOIN genes b ON a.interval INTERSECTS b.interval", + tables=["peaks", "genes"], + dialect="duckdb", + ) + + # Act + rows = sorted(conn.execute(sql).fetchall(), key=_null_key) + + # Assert — a RIGHT join preserves genes, so the reference runs mirrored. + # The path assertion is load-bearing: the naive plan returns these + # same rows, so without it this test passes even if the + # decomposition silently stops firing. + assert sql.count("SET VARIABLE __giql_iejoin_") == 2 + assert rows == _python_left_join_overlap(gene_rows, peak_rows) + + def test_transpile_should_decline_outer_join_when_full_outer(self): + """Test that a FULL OUTER join still falls back to the naive plan. + + Given: + A ``FULL OUTER JOIN`` whose ON carries a column-to-column + INTERSECTS. + When: + ``transpile`` is called with ``dialect='duckdb'``. + Then: + It should decline to the naive-predicate plan, since preserving + unmatched rows on *both* sides has no two-half decomposition. + """ + # Arrange & act + sql = transpile( + "SELECT a.chrom, b.chrom AS b_chrom FROM peaks a " + "FULL OUTER JOIN genes b ON a.interval INTERSECTS b.interval", + tables=["peaks", "genes"], + dialect="duckdb", + ) + + # Assert + assert "SET VARIABLE __giql_iejoin_" not in sql + assert 'a."chrom" = b."chrom"' in sql + + def test_transpile_should_decline_outer_join_when_projection_is_expression(self): + """Test that an expression projection under an outer join declines. + + Given: + A LEFT-join INTERSECTS whose SELECT list contains a ``CASE`` + expression (the ``bedtools -wao`` shape). + When: + ``transpile`` is called with ``dialect='duckdb'``. + Then: + It should decline to the naive-predicate plan — the unmatched half + can only NULL-fill a side-attributable column, not an arbitrary + expression (this shape unblocks with #109). + """ + # Arrange & act + sql = transpile( + "SELECT a.chrom, CASE WHEN b.chrom IS NULL THEN 0 ELSE 1 END AS ov " + "FROM peaks a LEFT JOIN genes b ON a.interval INTERSECTS b.interval", + tables=["peaks", "genes"], + dialect="duckdb", + ) + + # Assert + assert "SET VARIABLE __giql_iejoin_" not in sql + assert 'a."chrom" = b."chrom"' in sql + + def test_transpile_should_decline_outer_join_when_order_by(self): + """Test that a top-level ORDER BY under an outer join declines. + + Given: + A LEFT-join INTERSECTS carrying a top-level ``ORDER BY``. + When: + ``transpile`` is called with ``dialect='duckdb'``. + Then: + It should decline to the naive-predicate plan, because the modifier + belongs on the union rather than on each half independently. + """ + # Arrange & act + sql = transpile( + "SELECT a.chrom, a.start FROM peaks a " + "LEFT JOIN genes b ON a.interval INTERSECTS b.interval " + "ORDER BY a.start", + tables=["peaks", "genes"], + dialect="duckdb", + ) + + # Assert + assert "SET VARIABLE __giql_iejoin_" not in sql + assert 'a."chrom" = b."chrom"' in sql + + def test_transpile_should_route_count_overlaps_to_zero_fill_not_decomposition(self): + """Test that count_overlaps keeps its own faster zero-fill path. + + Given: + A ``COUNT(b.col)`` LEFT-join with a GROUP BY — also an outer join, + so both fast paths could plausibly claim it. + When: + ``transpile`` is called with ``dialect='duckdb'``. + Then: + It should emit the ``__giql_counts`` zero-fill wrapper and no + ``__giql_unmatched`` half, proving the dispatch order still + favours the count_overlaps path. + """ + # Arrange & act + sql = transpile( + 'SELECT a.chrom, a.start, a."end", COUNT(b.chrom) AS n ' + "FROM peaks a LEFT JOIN genes b ON a.interval INTERSECTS b.interval " + 'GROUP BY a.chrom, a.start, a."end"', + tables=["peaks", "genes"], + dialect="duckdb", + ) + + # Assert + assert "__giql_counts" in sql + assert "__giql_unmatched" not in sql + + @settings( + max_examples=50, + deadline=None, + suppress_health_check=[HealthCheck.function_scoped_fixture], + ) + @given( + peaks=st.lists( + st.tuples( + st.sampled_from(["chr1", "chr2", "chr3", None]), + st.integers(min_value=0, max_value=100), + st.integers(min_value=0, max_value=30), + ), + min_size=0, + max_size=8, + ), + genes=st.lists( + st.tuples( + st.sampled_from(["chr1", "chr2", "chr3", None]), + st.integers(min_value=0, max_value=100), + st.integers(min_value=0, max_value=30), + ), + min_size=0, + max_size=8, + ), + ) + def test_left_join_should_match_python_reference_for_random_inputs( + self, conn, peaks, genes + ): + """Test the outer-join decomposition against a Python-native reference. + + Given: + A Hypothesis-generated pair of small interval lists, which freely + produce duplicate rows, empty tables, and chromosomes present on + only one side. + When: + A LEFT-join INTERSECTS is transpiled with ``dialect='duckdb'`` and + executed. + Then: + The returned rows should equal the Python LEFT-join reference as a + multiset, including unmatched rows carrying NULL right columns. + """ + # Arrange + peak_rows = [(c, s, s + length) for (c, s, length) in peaks] + gene_rows = [(c, s, s + length) for (c, s, length) in genes] + _make_interval_tables(conn, peak_rows, gene_rows) + sql = transpile( + 'SELECT a.chrom, a.start, a."end", b.chrom AS b_chrom, ' + 'b.start AS b_start, b."end" AS b_end ' + "FROM peaks a LEFT JOIN genes b ON a.interval INTERSECTS b.interval", + tables=["peaks", "genes"], + dialect="duckdb", + ) + + # Act + rows = sorted(conn.execute(sql).fetchall(), key=_null_key) + + # Assert + # The path assertion is load-bearing: the naive plan returns these + # same rows, so without it this test passes even if the + # decomposition silently stops firing. + assert sql.count("SET VARIABLE __giql_iejoin_") == 2 + assert rows == _python_left_join_overlap(peak_rows, gene_rows) + + @pytest.mark.parametrize( + "query", + [ + "SELECT a.chrom AS Start, a.start, b.chrom AS bc FROM peaks a " + "LEFT JOIN genes b ON a.interval INTERSECTS b.interval", + "SELECT b.chrom AS y, b.start AS Y, a.start FROM peaks a " + "RIGHT JOIN genes b ON a.interval INTERSECTS b.interval", + ], + ids=["left", "right"], + ) + def test_transpile_should_decline_when_preserved_names_collide_case_insensitively( + self, conn, query + ): + """Test that case-only colliding preserved output names decline. + + Given: + An outer-join INTERSECTS whose preserved-side projections have + output names differing only in letter case. + When: + The query is transpiled with ``dialect='duckdb'`` and executed. + Then: + It should decline to the naive-predicate plan and return that + plan's rows and column types unchanged. + + DuckDB resolves identifiers case-insensitively even when quoted, so the + unmatched half's by-name re-projection would bind both positions to + whichever column came first -- returning the chromosome where a start + was asked for, and flipping that column's type to VARCHAR for the + matched rows too. + """ + # Arrange + _make_interval_tables( + conn, [("chr1", 10, 20), ("chr1", 900, 910)], [("chr1", 15, 25)] + ) + + # Act & assert + sql = _assert_agrees_with_naive(conn, query) + + # Assert + assert "SET VARIABLE __giql_iejoin_" not in sql + + @pytest.mark.parametrize( + ("peaks", "genes", "query"), + [ + ( + [("chr1", 100, 200), (None, 10, 20)], + [("chr1", 150, 250)], + 'SELECT a.chrom, a.start, a."end", b.chrom AS bc FROM peaks a ' + "LEFT JOIN genes b ON a.interval INTERSECTS b.interval", + ), + ( + [(None, 1, 10), (None, 20, 30)], + [("chr1", 150, 250)], + 'SELECT a.chrom, a.start, a."end", b.chrom AS bc FROM peaks a ' + "LEFT JOIN genes b ON a.interval INTERSECTS b.interval", + ), + ( + [("chr1", 100, 200)], + [("chr1", 150, 250), (None, 1, 2)], + "SELECT b.chrom, b.start, a.chrom AS ac FROM peaks a " + "RIGHT JOIN genes b ON a.interval INTERSECTS b.interval", + ), + ], + ids=["left_one_null_chrom", "left_all_null_chrom", "right_null_chrom"], + ) + def test_query_should_preserve_rows_whose_chromosome_is_null( + self, conn, peaks, genes, query + ): + """Test that a preserved-side row with a NULL chromosome still surfaces. + + Given: + An outer-join INTERSECTS where the row-preserving side contains a + NULL chromosome, including the case where every row does. + When: + The query is transpiled with ``dialect='duckdb'`` and executed. + Then: + It should return the naive plan's rows, with the NULL-chromosome + rows present and NULL-filled on the other side. + + Neither half can surface these on its own: both partitions come from + ``SELECT DISTINCT ``, where a NULL renders as a NULL literal that + ``string_agg`` skips, so no per-chromosome branch is ever emitted for + it. Dropping them would defeat the one guarantee an outer join makes. + """ + # Arrange + _make_interval_tables(conn, peaks, genes) + + # Act & assert + sql = _assert_agrees_with_naive(conn, query) + + # Assert + assert sql.count("SET VARIABLE __giql_iejoin_") == 2 + + @pytest.mark.parametrize( + "clause", + [ + "QUALIFY ROW_NUMBER() OVER (PARTITION BY a.chrom ORDER BY a.start) = 1", + "LIMIT 2", + "LIMIT 2 OFFSET 1", + "GROUP BY a.chrom, a.start", + "ORDER BY a.start", + ], + ids=["qualify", "limit", "limit_offset", "group_by", "order_by"], + ) + def test_transpile_should_decline_outer_join_when_top_level_clause_present( + self, conn, clause + ): + """Test that any unread top-level clause declines the decomposition. + + Given: + A LEFT-join INTERSECTS carrying a top-level clause the rewrite does + not itself consume. + When: + The query is transpiled with ``dialect='duckdb'`` and executed. + Then: + It should decline to the naive-predicate plan and match its rows. + + The rewrite re-emits the query as a UNION ALL of two independently + transpiled halves, so any such clause would be applied per half rather + than over the union -- ``LIMIT 2`` would return four rows, not two. + """ + # Arrange + _make_interval_tables( + conn, + [("chr1", 100, 200), ("chr1", 5000, 6000), ("chrX", 10, 20)], + [("chr1", 150, 250), ("chr1", 180, 300)], + ) + query = ( + "SELECT a.chrom, a.start FROM peaks a " + f"LEFT JOIN genes b ON a.interval INTERSECTS b.interval {clause}" + ) + + # Act & assert + sql = _assert_agrees_with_naive(conn, query) + + # Assert + assert "SET VARIABLE __giql_iejoin_" not in sql + + @pytest.mark.parametrize( + ("label", "query"), + [ + ( + "from_operand", + "SELECT a.chrom, b.start AS bs FROM peaks a TABLESAMPLE 100 PERCENT " + "LEFT JOIN genes b ON a.interval INTERSECTS b.interval", + ), + ( + "joined_operand", + "SELECT a.chrom, b.start AS bs FROM peaks a " + "LEFT JOIN genes b TABLESAMPLE 100 PERCENT " + "ON a.interval INTERSECTS b.interval", + ), + ], + ) + def test_transpile_should_decline_outer_join_when_operand_is_sampled( + self, conn, label, query + ): + """Test that a TABLESAMPLE on either operand declines the decomposition. + + Given: + A LEFT-join INTERSECTS whose FROM table or whose joined table + carries ``TABLESAMPLE``. + When: + The query is transpiled with ``dialect='duckdb'`` and executed. + Then: + It should decline to the naive-predicate plan and remain executable. + + The sample rides on the table node rather than the top-level SELECT, so + the clause whitelist cannot see it; left alone it lands in a position + the per-chromosome template cannot parse at all. Each operand is gated + separately, so each needs its own case. + """ + # Arrange + _make_interval_tables(conn, [("chr1", 100, 200)], [("chr1", 150, 250)]) + + # Act & assert + sql = _assert_agrees_with_naive(conn, query) + + # Assert + assert "SET VARIABLE __giql_iejoin_" not in sql + + @pytest.mark.parametrize( + "query", + [ + 'SELECT a.name AS "we;\nird", b.name AS bn FROM peaks a ' + "LEFT JOIN genes b ON a.interval INTERSECTS b.interval", + 'SELECT a.name AS "we;\nird", COUNT(b.name) AS n FROM peaks a ' + "LEFT JOIN genes b ON a.interval INTERSECTS b.interval " + "GROUP BY a.name", + ], + ids=["decomposition", "count_overlaps"], + ) + def test_query_should_handle_identifier_containing_statement_separator( + self, conn, query + ): + """Test that an identifier containing the statement separator still works. + + Given: + An outer-join INTERSECTS whose output alias contains ``";\\n"`` -- + the sequence separating the emitted script's statements. + When: + The query is transpiled with ``dialect='duckdb'`` and executed. + Then: + It should execute and match the naive plan, rather than splitting + the script inside the quoted alias and failing to parse. + + Both composing rewrites take the setup and SELECT as separate values + now; recovering them by splitting rendered SQL is what broke here. + """ + # Arrange + _make_table(conn, "peaks", [("chr1", 100, 200, "p1", 0, "+")]) + _make_table(conn, "genes", [("chr1", 150, 250, "g1", 0, "+")]) + + # Act & assert + _assert_agrees_with_naive(conn, query) + + def test_query_should_agree_with_naive_when_projection_interleaves_sides(self, conn): + """Test that a projection alternating sides keeps every column in place. + + Given: + A LEFT join projecting preserved and NULL-filled columns in + alternating positions, over data covering matched rows, unmatched + rows and a NULL chromosome -- one row for each of the three branches. + When: + The query is transpiled with ``dialect='duckdb'`` and executed. + Then: + It should return the naive plan's rows, column names and types. + + Every branch re-projects by position, walking the preserved columns with + a running index while the NULL fills consume none of it. A projection + that groups the preserved columns first -- which most shapes here do -- + keeps that index correct even if the arms are mis-sequenced, so only an + interleaved projection can catch it. + """ + # Arrange + _make_table( + conn, + "peaks", + [ + ("chr1", 100, 200, "p1", 0, "+"), + ("chr1", 5000, 6000, "p2", 0, "+"), + (None, 100, 200, "pnull", 0, "+"), + ], + ) + _make_table(conn, "genes", [("chr1", 150, 250, "g1", 0, "+")]) + query = ( + "SELECT a.chrom, b.start AS bs, a.start, b.chrom AS bc, a.name " + "FROM peaks a LEFT JOIN genes b ON a.interval INTERSECTS b.interval" + ) + + # Act & assert + sql = _assert_agrees_with_naive(conn, query) + + # Assert + assert sql.count("SET VARIABLE __giql_iejoin_") == 2 + + def test_transform_to_parts_should_return_setup_and_select_separately(self): + """Test that the primitive hands back the two parts unjoined. + + Given: + A decomposed outer join and a transformer over its tables. + When: + transform_to_parts is called. + Then: + It should return the setup statements and the final SELECT as a + pair, so a rewrite composing on top never has to recover them by + splitting rendered SQL. + + Splitting is what the pair exists to avoid: the separator appears inside + the script itself whenever an identifier contains it. + """ + # Arrange + tables = Tables() + for name in ("peaks", "genes"): + tables.register(name, Table(name)) + transformer = IntersectsDuckDBIEJoinTransformer(tables) + ast = parse_one( + "SELECT a.chrom, a.start FROM peaks a " + "LEFT JOIN genes b ON a.interval INTERSECTS b.interval", + dialect=GIQLDialect, + ) + + # Act + setup, select = transformer.transform_to_parts(ast) + + # Assert + assert [s for s in setup.split(";\n") if not s.startswith("SET VARIABLE")] == [] + assert select.startswith("SELECT ") + assert "SET VARIABLE" not in select + + def test_transform_to_parts_should_not_mutate_its_query(self): + """Test that the primitive leaves the AST it is handed unchanged. + + Given: + A parsed decomposed outer join. + When: + transform_to_parts is called on it. + Then: + The AST should be unchanged. + + The registry entry point passes the live statement AST that pass 3 goes + on transforming, and the naive-predicate fallback rewrites that same node + in place when this path declines. Every composed rewrite here now + transpiles a copy rather than the caller's node, which is what lets this + hold without each of them defending it. + """ + # Arrange + tables = Tables() + for name in ("peaks", "genes"): + tables.register(name, Table(name)) + transformer = IntersectsDuckDBIEJoinTransformer(tables) + ast = parse_one( + "SELECT a.chrom, a.start FROM peaks a " + "LEFT JOIN genes b ON a.interval INTERSECTS b.interval", + dialect=GIQLDialect, + ) + before = repr(ast) + + # Act + transformer.transform_to_parts(ast) + + # Assert + assert repr(ast) == before + + @pytest.mark.parametrize( + "side, raises", + [("", True), ("LEFT ", False), ("RIGHT ", False)], + ids=["inner", "left", "right"], + ) + def test_transpile_should_apply_its_documented_projection_policy_per_join_side( + self, side, raises + ): + """Test that an unqualified projection raises on INNER and declines on outer. + + Given: + One unqualified projection, spelled as an INNER, a LEFT and a RIGHT + join. + When: + Each is transpiled with ``dialect='duckdb'``. + Then: + INNER should raise ``ValueError`` and the outer forms should decline + to the naive plan, returning its rows. + + The two policies are documented separately and reached by different + code: the INNER family raises from projection resolution, while the + outer-join matcher returns None for a column it cannot attribute to a + side. Before LEFT and RIGHT joined this dialect they were not IEJoin + shapes at all, so the divergence was invisible. Pinning both sides + against one projection is what stops the documented contract and the + behaviour drifting apart again. + """ + # Arrange + query = ( + f"SELECT chrom, b.start FROM peaks a {side}JOIN genes b " + "ON a.interval INTERSECTS b.interval" + ) + + # Act & assert -- transpile-level, because the naive plan for this + # projection is itself ambiguous SQL the engine rejects at bind time. + # What the contract governs is which of the two policies applies, and + # that is decided before any of it reaches DuckDB. + if raises: + with pytest.raises(ValueError, match="qualified projections"): + transpile(query, tables=["peaks", "genes"], dialect="duckdb") + return + sql = transpile(query, tables=["peaks", "genes"], dialect="duckdb") + + # Assert + assert "SET VARIABLE __giql_iejoin_" not in sql + assert sql == transpile(query, tables=["peaks", "genes"]) + + def test_transpile_should_qualify_null_chrom_branch_with_the_rendered_alias(self): + """Test that the NULL-chromosome branch qualifies with its own FROM alias. + + Given: + A LEFT join whose preserved side carries a quoted mixed-case alias. + When: + The query is transpiled with ``dialect='duckdb'``. + Then: + The NULL-chromosome branch's WHERE qualifier should be byte-equal to + the alias its own FROM clause renders. + + The branch used to render ``FROM peaks AS "A" ... WHERE a.chrom``, mixing + the original case with the case-folded alias. That bound to one relation + only because DuckDB folds identifiers, quoted ones included -- a property + the adjacent identifier helper's docstring actively denies. Correct by + accident is worth pinning as correct by construction. + """ + # Arrange + query = ( + 'SELECT "A".chrom, b.start FROM peaks "A" ' + 'LEFT JOIN genes b ON "A".interval INTERSECTS b.interval' + ) + + # Act + sql = transpile(query, tables=["peaks", "genes"], dialect="duckdb") + branch = sql[sql.rindex("UNION ALL") :] + + # Assert + rendered_from = re.search(r"FROM peaks AS (\S+) WHERE", branch) + rendered_where = re.search(r"WHERE (\S+)\.chrom IS NULL", branch) + assert rendered_from is not None and rendered_where is not None + assert rendered_where.group(1) == rendered_from.group(1) + + def test_query_should_agree_with_naive_when_preserved_alias_is_mixed_case( + self, conn + ): + """Test that a mixed-case preserved alias returns the naive plan's rows. + + Given: + A LEFT join whose preserved side carries a quoted mixed-case alias, + over data including rows whose chromosome is NULL -- the rows only + the third branch surfaces. + When: + The query is transpiled with ``dialect='duckdb'`` and executed. + Then: + It should return the naive plan's rows, column names and column + types. + """ + # Arrange + _make_table( + conn, + "peaks", + [ + ("chr1", 100, 200, "p1", 0, "+"), + ("chr1", 5000, 6000, "p2", 0, "+"), + (None, 100, 200, "pnull", 0, "+"), + ], + ) + _make_table(conn, "genes", [("chr1", 150, 250, "g1", 0, "+")]) + query = ( + 'SELECT "A".chrom, "A".start, b.start AS bs FROM peaks "A" ' + 'LEFT JOIN genes b ON "A".interval INTERSECTS b.interval' + ) + + # Act & assert + sql = _assert_agrees_with_naive(conn, query) + + # Assert + assert sql.count("SET VARIABLE __giql_iejoin_") == 2 + + @pytest.mark.parametrize( + "shape", + [ + pytest.param("three_tables", id="three_tables"), + pytest.param("table_function_operand", id="table_function_operand"), + ], + ) + @pytest.mark.parametrize("side", ["", "LEFT "], ids=["inner", "left"]) + def test_transpile_should_decline_malformed_shape_on_every_join_side( + self, conn, shape, side + ): + """Test that a malformed two-table shape declines whatever the join side. + + Given: + A shape the shared operand resolver rejects -- a third joined table, + or a table-function operand that parses with an empty name -- + spelled once as an INNER join and once as a LEFT join. + When: + Each is transpiled with ``dialect='duckdb'`` and executed. + Then: + Both should decline to the naive plan and return its rows. + + The INNER path and the outer-join matchers resolve their operands + through one shared helper, so a gap in it would open on both at once. + Pinning both sides is what makes that shared gate observable: before the + merge each path carried its own copy, and a fix to one left the other. + """ + # Arrange + _make_table( + conn, + "peaks", + [("chr1", 100, 200, "p1", 0, "+"), ("chr1", 5000, 6000, "p2", 0, "+")], + ) + _make_table(conn, "genes", [("chr1", 150, 250, "g1", 0, "+")]) + _make_table(conn, "exons", [("chr1", 150, 250, "e1", 0, "+")]) + queries = { + "three_tables": ( + f"SELECT a.chrom, b.start FROM peaks a {side}JOIN genes b " + "ON a.interval INTERSECTS b.interval JOIN exons c ON b.chrom = c.chrom" + ), + "table_function_operand": ( + f"SELECT a.chrom, b.start FROM peaks a {side}JOIN DISJOIN(genes) b " + "ON a.interval INTERSECTS b.interval" + ), + } + + # Act & assert + sql = _assert_agrees_with_naive(conn, queries[shape]) + + # Assert + assert "SET VARIABLE __giql_iejoin_" not in sql + + @pytest.mark.parametrize( + ("label", "query"), + [ + ( + "duplicate_preserved_names", + "SELECT a.chrom AS c, a.start AS c, b.start AS bs FROM peaks a " + "LEFT JOIN genes b ON a.interval INTERSECTS b.interval", + ), + ( + "where_alongside_on_intersects", + "SELECT a.chrom, b.start AS bs FROM peaks a " + "LEFT JOIN genes b ON a.interval INTERSECTS b.interval " + "WHERE a.start > 1", + ), + ( + "qualified_star", + "SELECT a.*, b.start AS bs FROM peaks a " + "LEFT JOIN genes b ON a.interval INTERSECTS b.interval", + ), + ( + "self_join", + "SELECT a.chrom, b.start AS bs FROM peaks a " + "LEFT JOIN peaks b ON a.interval INTERSECTS b.interval", + ), + ( + "on_residual", + "SELECT a.chrom, b.start AS bs FROM peaks a " + "LEFT JOIN genes b ON a.interval INTERSECTS b.interval " + "AND a.start > 1", + ), + ( + "repeated_intersects_in_on", + "SELECT a.chrom, b.start AS bs FROM peaks a " + "LEFT JOIN genes b ON a.interval INTERSECTS b.interval " + "AND a.interval INTERSECTS b.interval", + ), + ( + "subquery_from_operand", + "SELECT a.chrom, b.start AS bs FROM (SELECT * FROM peaks) a " + "LEFT JOIN genes b ON a.interval INTERSECTS b.interval", + ), + ( + "subquery_joined_operand", + "SELECT a.chrom, b.start AS bs FROM peaks a " + "LEFT JOIN (SELECT * FROM genes) b " + "ON a.interval INTERSECTS b.interval", + ), + ], + ) + def test_transpile_should_decline_outer_join_for_unsupported_shape( + self, conn, label, query + ): + """Test that each unsupported outer-join shape declines as one unit. + + Given: + An outer-join INTERSECTS in a shape the decomposition does not + support -- colliding preserved names, a WHERE alongside the ON + INTERSECTS, a qualified star, a self-join, an ON residual, a + repeated INTERSECTS, or a subquery in place of a base table on + either side. + When: + The query is transpiled with ``dialect='duckdb'`` and executed. + Then: + It should emit no session variable at all and return the naive + plan's rows, never a half-emitted script or a raised error. + """ + # Arrange + _make_table( + conn, + "peaks", + [("chr1", 100, 200, "p1", 0, "+"), ("chr1", 5000, 6000, "p2", 0, "+")], + ) + _make_table(conn, "genes", [("chr1", 150, 250, "g1", 0, "+")]) + + # Act & assert + sql = _assert_agrees_with_naive(conn, query) + + # Assert + assert "SET VARIABLE __giql_iejoin_" not in sql + + @pytest.mark.parametrize("kind", ["LEFT SEMI", "LEFT ANTI"]) + def test_transpile_should_not_decompose_left_semi_or_anti_join(self, kind): + """Test that LEFT SEMI / LEFT ANTI joins are never decomposed. + + Given: + A ``LEFT SEMI`` or ``LEFT ANTI`` join carrying a column-to-column + INTERSECTS. sqlglot parses these with ``side='LEFT'``, so they clear + the matcher's side gate and reach its ``kind`` gate -- but the join + preserves no unmatched rows. + When: + The query is transpiled with ``dialect='duckdb'``. + Then: + It should not emit the two-half decomposition, which would answer a + semi or anti join as an outer join. + """ + # Act + sql = transpile( + f"SELECT a.chrom, a.start FROM peaks a {kind} JOIN genes b " + "ON a.interval INTERSECTS b.interval", + tables=["peaks", "genes"], + dialect="duckdb", + ) + + # Assert + # The decomposition is the only shape emitting a second SET VARIABLE, so + # this fails if the kind gate stops declining -- unlike a bare + # "__giql_unmatched" check, which the single-variable path can never + # produce whether the gate holds or not. + assert sql.count("SET VARIABLE __giql_iejoin_") <= 1 + assert "__giql_unmatched" not in sql + + @pytest.mark.parametrize("kind", ["SEMI", "ANTI"]) + def test_transpile_should_not_decompose_unsided_semi_or_anti_join(self, kind): + """Test that bare SEMI / ANTI joins are never decomposed. + + Given: + A bare ``SEMI`` or ``ANTI`` join carrying a column-to-column + INTERSECTS. sqlglot parses these with ``side=None``, so they are + declined by the matcher's side gate rather than its kind gate, and + they keep the single-variable SEMI / ANTI IEJoin path. + When: + The query is transpiled with ``dialect='duckdb'``. + Then: + It should emit the single-variable path and never the unmatched + half. + """ + # Act + sql = transpile( + f"SELECT a.chrom, a.start FROM peaks a {kind} JOIN genes b " + "ON a.interval INTERSECTS b.interval", + tables=["peaks", "genes"], + dialect="duckdb", + ) + + # Assert + assert sql.count("SET VARIABLE __giql_iejoin_") == 1 + assert "__giql_unmatched" not in sql + + @pytest.mark.parametrize( + "query", + [ + "SELECT a.chrom, b.start AS bs FROM peaks a " + "LEFT OUTER JOIN genes b ON a.interval INTERSECTS b.interval", + "SELECT b.chrom, a.start AS asx FROM peaks a " + "RIGHT OUTER JOIN genes b ON a.interval INTERSECTS b.interval", + "SELECT (a.chrom), b.start AS bs FROM peaks a " + "LEFT JOIN genes b ON a.interval INTERSECTS b.interval", + "SELECT b.start AS bs, a.chrom, b.chrom AS bc FROM peaks a " + "LEFT JOIN genes b ON a.interval INTERSECTS b.interval", + ], + ids=["left_outer", "right_outer", "parenthesized", "interleaved_sides"], + ) + def test_query_should_decompose_supported_outer_join_spellings(self, conn, query): + """Test that supported outer-join spellings decompose and stay correct. + + Given: + An outer-join INTERSECTS written as explicit ``LEFT OUTER`` / + ``RIGHT OUTER``, with a parenthesized projection, or with the two + sides interleaved in the SELECT list. + When: + The query is transpiled with ``dialect='duckdb'`` and executed. + Then: + It should take the two-half decomposition and match the naive + plan's rows and column types. + """ + # Arrange + _make_interval_tables( + conn, + [("chr1", 100, 200), ("chr1", 5000, 6000), ("chrX", 10, 20)], + [("chr1", 150, 250), ("chrY", 1, 2)], + ) + + # Act & assert + sql = _assert_agrees_with_naive(conn, query) + + # Assert + assert sql.count("SET VARIABLE __giql_iejoin_") == 2 + + def test_query_should_use_configured_chrom_column_for_other_side_projection( + self, conn + ): + """Test that the unmatched half honours a custom chromosome column. + + Given: + Tables registered with non-default chrom/start/end column names, and + a LEFT-join projecting only other-side columns -- the one path where + the builder must synthesise a preserved-side chromosome reference. + When: + The query is transpiled with ``dialect='duckdb'`` and executed. + Then: + It should resolve that column from the preserved table's config and + match the naive plan, rather than assuming the default name. + """ + # Arrange + for table in ("cpeaks", "cgenes"): + conn.execute( + f"CREATE TABLE {table} (contig VARCHAR, beg INTEGER, stop INTEGER)" + ) + conn.executemany( + "INSERT INTO cpeaks VALUES (?, ?, ?)", + [("chr1", 100, 200), ("chr1", 5000, 6000)], + ) + conn.executemany("INSERT INTO cgenes VALUES (?, ?, ?)", [("chr1", 150, 250)]) + tables = [ + Table("cpeaks", chrom_col="contig", start_col="beg", end_col="stop"), + Table("cgenes", chrom_col="contig", start_col="beg", end_col="stop"), + ] + query = ( + "SELECT b.contig AS bc, b.beg AS bb FROM cpeaks a " + "LEFT JOIN cgenes b ON a.interval INTERSECTS b.interval" + ) + + # Act + dialect_sql = transpile(query, tables=tables, dialect="duckdb") + naive_sql = transpile(query, tables=tables, dialect=None) + + # Assert + assert dialect_sql.count("SET VARIABLE __giql_iejoin_") == 2 + assert _rows_and_schema(conn, dialect_sql) == _rows_and_schema(conn, naive_sql) + + def test_query_should_plan_both_decomposition_halves_through_ie_join(self, conn): + """Test that the matched and unmatched halves both reach IE_JOIN. + + Given: + A LEFT-join INTERSECTS over enough rows for the planner to choose a + range join, with a share of left rows unmatched. + When: + Each emitted session variable's dynamic SQL is planned separately. + Then: + Both halves should plan through ``IE_JOIN`` and neither through the + quadratic ``BLOCKWISE_NL_JOIN``. + + Planning only the first variable -- the helper's old behaviour -- would + report nothing at all about the unmatched half. + """ + # Arrange + conn.execute("DROP TABLE IF EXISTS peaks") + conn.execute(""" + CREATE TABLE peaks AS + SELECT 'chr' || ((i % 4) + 1) AS chrom, + ((i * 37) % 100000)::INTEGER AS "start", + (((i * 37) % 100000) + 200)::INTEGER AS "end" + FROM range(20000) t(i) + """) + conn.execute("DROP TABLE IF EXISTS genes") + conn.execute(""" + CREATE TABLE genes AS + SELECT 'chr' || ((i % 4) + 1) AS chrom, + ((i * 41 + 17) % 100000)::INTEGER AS "start", + (((i * 41 + 17) % 100000) + 200)::INTEGER AS "end" + FROM range(20000) t(i) + WHERE (i % 4) + 1 <= 3 + """) + sql = transpile( + "SELECT a.chrom, a.start, b.start AS bs FROM peaks a " + "LEFT JOIN genes b ON a.interval INTERSECTS b.interval", + tables=["peaks", "genes"], + dialect="duckdb", + ) + + # Act + matched_plan = _explain_dynamic_sql(conn, sql, half=0) + unmatched_plan = _explain_dynamic_sql(conn, sql, half=1) + + # Assert + assert len(_dynamic_sql_variables(sql)) == 2 + for plan in (matched_plan, unmatched_plan): + assert "IE_JOIN" in plan + assert "BLOCKWISE_NL_JOIN" not in plan + + def test_query_should_bound_session_variables_across_repeated_execution(self, conn): + """Test that repeating one query does not grow the session's variable set. + + Given: + A decomposed outer join transpiled and executed 25 times on one + connection. + When: + DuckDB's ``duckdb_variables()`` catalog is counted. + Then: + It should hold exactly the two variables the query declares, not one + pair per execution. + + This is the accumulation bound itself rather than a proxy for it: with a + fresh name per call the same loop leaves 50 entries, each holding the + full generated per-chromosome SQL, reclaimable only by dropping the + connection. + """ + # Arrange + _make_interval_tables( + conn, + [("chr1", 100, 200), ("chr1", 5000, 6000), ("chrX", 10, 20)], + [("chr1", 150, 250)], + ) + query = ( + "SELECT a.chrom, a.start, b.start AS bs FROM peaks a " + "LEFT JOIN genes b ON a.interval INTERSECTS b.interval" + ) + + # Act + for _ in range(25): + conn.execute( + transpile(query, tables=["peaks", "genes"], dialect="duckdb") + ).fetchall() + + # Assert + declared = conn.execute( + "SELECT count(*) FROM duckdb_variables() WHERE name LIKE '__giql_iejoin_%'" + ).fetchone()[0] + assert declared == 2 + + def test_query_should_isolate_session_variables_across_interleaved_queries( + self, conn + ): + """Test that two decomposed queries do not rebind each other's variables. + + Given: + Two different decomposed outer joins transpiled against the same + connection, each declaring its own pair of session variables. + When: + Their setup statements are interleaved before either final SELECT + runs, and the first query is then re-executed. + Then: + Each should return its own rows unchanged. + + Session variables are global DuckDB state, and this rewrite now declares + two per query, so a token collision would silently cross-wire results. + """ + # Arrange + _make_interval_tables( + conn, + [("chr1", 100, 200), ("chr1", 5000, 6000), ("chrX", 10, 20)], + [("chr1", 150, 250)], + ) + left_sql = transpile( + "SELECT a.chrom, a.start, b.start AS bs FROM peaks a " + "LEFT JOIN genes b ON a.interval INTERSECTS b.interval", + tables=["peaks", "genes"], + dialect="duckdb", + ) + right_sql = transpile( + "SELECT b.chrom, b.start, a.start AS asx FROM peaks a " + "RIGHT JOIN genes b ON a.interval INTERSECTS b.interval", + tables=["peaks", "genes"], + dialect="duckdb", + ) + left_statements = [s for s in left_sql.split(";\n") if s.strip()] + right_statements = [s for s in right_sql.split(";\n") if s.strip()] + expected_left = sorted(conn.execute(left_sql).fetchall(), key=_null_key) + expected_right = sorted(conn.execute(right_sql).fetchall(), key=_null_key) + + # Act -- interleave every setup statement before either final SELECT. + for left_statement, right_statement in zip( + left_statements[:-1], right_statements[:-1] + ): + conn.execute(left_statement) + conn.execute(right_statement) + actual_right = sorted( + conn.execute(right_statements[-1]).fetchall(), key=_null_key + ) + actual_left = sorted(conn.execute(left_statements[-1]).fetchall(), key=_null_key) + + # Assert + assert actual_left == expected_left + assert actual_right == expected_right + + @pytest.mark.parametrize( + ("peaks", "genes"), + [ + ([("chr1", 100, 200)], []), + ([], [("chr1", 150, 250)]), + ([], []), + ([("chr1", 100, 200)], [("chrY", 1, 2)]), + ], + ids=["empty_right", "empty_left", "both_empty", "disjoint_chromosomes"], + ) + def test_query_should_preserve_column_types_when_a_half_is_empty( + self, conn, peaks, genes + ): + """Test that NULL fills keep their declared types when a half is empty. + + Given: + An outer-join INTERSECTS where one or both tables are empty, or the + two share no chromosome, so a half contributes no rows. + When: + The query is transpiled with ``dialect='duckdb'`` and executed. + Then: + It should return the naive plan's rows and its column types. + + The NULL fills take their type from the first UNION ALL branch, so an + empty matched half is where a type could silently degrade. + """ + # Arrange + _make_interval_tables(conn, peaks, genes) + query = ( + 'SELECT a.chrom, a.start, a."end", b.chrom AS bc, b.start AS bs ' + "FROM peaks a LEFT JOIN genes b ON a.interval INTERSECTS b.interval" + ) + + # Act & assert + sql = _assert_agrees_with_naive(conn, query) + + # Assert + assert sql.count("SET VARIABLE __giql_iejoin_") == 2 + + def test_query_should_stay_subquadratic_when_left_join_at_scale(self, conn): + """Test that the decomposition still scales at 2^18 rows per side. + + Given: + 262,144 intervals per side spread over 24 chromosomes. + When: + The LEFT-join INTERSECTS query is transpiled with + ``dialect='duckdb'`` and executed. + Then: + It should return every left row at least once and finish well + inside the bound below. + + A plan assertion cannot cover this: DuckDB reports ``IE_JOIN`` for the + per-chromosome outer join this rewrite replaced, yet that plan needs + minutes at this size while the decomposition needs about a second. Only + executing at scale separates them, so the wall-clock bound is the + assertion — set ~50x above the expected runtime, far below the + quadratic alternative, so it discriminates without being flaky. + """ + # Arrange -- genes deliberately skip four of the 24 chromosomes so a + # sixth of the peaks have no overlap at all. Without that the matched + # half alone satisfies every assertion below and the unmatched half + # could be deleted wholesale without failing this test. + conn.execute("DROP TABLE IF EXISTS peaks") + conn.execute(""" + CREATE TABLE peaks AS + SELECT 'chr' || ((i % 24) + 1) AS chrom, + ((i * 37) % 1000000)::INTEGER AS "start", + (((i * 37) % 1000000) + 350)::INTEGER AS "end" + FROM range(262144) t(i) + """) + conn.execute("DROP TABLE IF EXISTS genes") + conn.execute(""" + CREATE TABLE genes AS + SELECT 'chr' || ((i % 24) + 1) AS chrom, + ((i * 37 + 173) % 1000000)::INTEGER AS "start", + (((i * 37 + 173) % 1000000) + 350)::INTEGER AS "end" + FROM range(262144) t(i) + WHERE (i % 24) + 1 <= 20 + """) + sql = transpile( + 'SELECT a.chrom, a.start, a."end", b.start AS b_start ' + "FROM peaks a LEFT JOIN genes b ON a.interval INTERSECTS b.interval", + tables=["peaks", "genes"], + dialect="duckdb", + ) + + # Act + started = time.perf_counter() + rows = conn.execute(sql).fetchall() + elapsed = time.perf_counter() - started + + # Assert -- every left row survives exactly as a distinct key, and the + # unmatched half genuinely contributes. A bare ``len(rows) >= 262144`` + # is satisfied by the matched half several times over and proves + # neither property. + assert len({row[:3] for row in rows}) == 262144 + assert sum(1 for row in rows if row[3] is None) > 0 + assert elapsed < 60 + + def test_query_should_not_regress_against_naive_when_preserved_side_has_many_contigs( + self, conn + ): + """Test that left-only contigs do not cost the shared contigs their fast plan. + + Given: + 200,000 intervals per side over 24 shared chromosomes, plus 5,000 + scaffolds carried only by the preserved side. + When: + The LEFT-join INTERSECTS query is transpiled both with + ``dialect='duckdb'`` and without, and both are executed. + Then: + The unmatched half should partition on the shared chromosomes, and + the decomposition should stay within a small multiple of the naive + plan's runtime. + + Partitioning the unmatched half on the preserved side's distinct + chromosomes makes its branch count track a number the join does not + depend on. The cost is indirect and easy to miss: those extra branches + push the partition over the cardinality ceiling, so the half abandons + ``IE_JOIN`` for *every* chromosome -- including the 24 that carry all + the rows. Measured here: 8.3s partitioning on preserved-distinct, 5.0s + on the shared INTERSECT, against 2.5s naive (#95). The bound is a ratio + rather than a wall clock so it calibrates to the machine. + """ + # Arrange + conn.execute("DROP TABLE IF EXISTS peaks") + conn.execute(""" + CREATE TABLE peaks AS + SELECT 'chr' || ((i % 24) + 1) AS chrom, + ((i * 37) % 1000000)::INTEGER AS "start", + (((i * 37) % 1000000) + 350)::INTEGER AS "end" + FROM range(200000) t(i) + """) + conn.execute(""" + INSERT INTO peaks + SELECT 'scaffold_' || (i / 3)::INTEGER, + (i % 3) * 100, + (i % 3) * 100 + 50 + FROM range(15000) t(i) + """) + conn.execute("DROP TABLE IF EXISTS genes") + conn.execute(""" + CREATE TABLE genes AS + SELECT 'chr' || ((i % 24) + 1) AS chrom, + ((i * 37 + 173) % 1000000)::INTEGER AS "start", + (((i * 37 + 173) % 1000000) + 350)::INTEGER AS "end" + FROM range(200000) t(i) + """) + query = ( + 'SELECT a.chrom, a.start, a."end", b.start AS b_start ' + "FROM peaks a LEFT JOIN genes b ON a.interval INTERSECTS b.interval" + ) + + # Act + dialect_sql = transpile(query, tables=["peaks", "genes"], dialect="duckdb") + dialect_elapsed, dialect_rows = _timed_rows(conn, dialect_sql) + naive_elapsed, naive_rows = _timed_rows( + conn, transpile(query, tables=["peaks", "genes"]) + ) + + # Assert -- the structural lock pins the mechanism even if the timing + # bound is ever loosened; the unmatched half is the second SET VARIABLE. + unmatched_setup = dialect_sql.split(";\n")[1] + assert "INTERSECT" in unmatched_setup + assert 'a."chrom" NOT IN (SELECT DISTINCT "chrom" FROM genes' in unmatched_setup + assert sorted(dialect_rows) == sorted(naive_rows) + assert dialect_elapsed < naive_elapsed * 2.5 + + def test_query_should_not_regress_against_naive_when_partition_exceeds_ceiling( + self, conn + ): + """Test that a partition above the ceiling degrades to the plain predicate. + + Given: + 200,000 intervals per side spread over 400 shared chromosomes -- + above ``MAX_PER_CHROM_PARTITIONS``. + When: + The LEFT-join INTERSECTS query is transpiled both with + ``dialect='duckdb'`` and without, and both are executed. + Then: + The decomposition should return the naive plan's rows and stay + within a small multiple of its runtime. + + The partition's cardinality is a data property no transpile-time check + can see, so the choice is made at execution time by a ``CASE`` over the + partition's own row count. Measured here at 400 shared contigs: 3.35s + before the guard (18.6x naive), 0.40s after (2.2x). + """ + # Arrange + for table, offset in (("peaks", 0), ("genes", 173)): + conn.execute(f"DROP TABLE IF EXISTS {table}") + conn.execute(f""" + CREATE TABLE {table} AS + SELECT 'chr' || (i % 400) AS chrom, + ((i * 37 + {offset}) % 500000)::INTEGER AS "start", + (((i * 37 + {offset}) % 500000) + 300)::INTEGER AS "end" + FROM range(200000) t(i) + """) + query = ( + 'SELECT a.chrom, a.start, a."end", b.start AS b_start ' + "FROM peaks a LEFT JOIN genes b ON a.interval INTERSECTS b.interval" + ) + + # Act + dialect_sql = transpile(query, tables=["peaks", "genes"], dialect="duckdb") + dialect_elapsed, dialect_rows = _timed_rows(conn, dialect_sql) + naive_elapsed, naive_rows = _timed_rows( + conn, transpile(query, tables=["peaks", "genes"]) + ) + + # Assert + assert f"> {MAX_PER_CHROM_PARTITIONS}" in dialect_sql + assert sorted(dialect_rows) == sorted(naive_rows) + assert dialect_elapsed < naive_elapsed * 8 + + class TestTranspileDuckDBIEJoinExecution: """End-to-end execution tests against in-memory DuckDB.""" diff --git a/tests/test_expander.py b/tests/test_expander.py index 6f7fa89..1c45469 100644 --- a/tests/test_expander.py +++ b/tests/test_expander.py @@ -1843,6 +1843,70 @@ def test_iejoin_query_should_emit_builtin_iejoin_without_override(self): # Assert assert "SET VARIABLE __giql_iejoin_" in sql + def test_declined_shape_should_expand_user_generic_override(self, clean_registry): + """Test that a generic Intersects override reaches shapes DuckDB declines. + + Given: + A user ``(GenericTarget, Intersects)`` expander registered alongside + the built-in DuckDB override, and a FULL OUTER join — a shape the + IEJoin path declines. + When: + Transpiling with dialect='duckdb' and with dialect=None. + Then: + Both should emit the user override's predicate. + + The DuckDB override called the built-in naive predicate directly, so the + two agreed only by coincidence: a user override took effect under + ``dialect=None`` and was silently ignored under ``dialect="duckdb"`` for + every shape the IEJoin path declines. Resolving the fallback through the + registry makes them the same expander by construction. + """ + # Arrange + clean_registry.register(DuckDBTarget(), Intersects, expand_intersects_duckdb) + clean_registry.register( + GenericTarget(), Intersects, lambda n, c: exp.condition("1 = 2") + ) + query = ( + "SELECT a.chrom, b.start FROM peaks a " + "FULL JOIN genes b ON a.interval INTERSECTS b.interval" + ) + + # Act + duckdb_sql = transpile(query, tables=["peaks", "genes"], dialect="duckdb") + generic_sql = transpile(query, tables=["peaks", "genes"]) + + # Assert + assert "1 = 2" in duckdb_sql + assert "1 = 2" in generic_sql + + def test_declined_shape_should_expand_when_generic_slot_is_empty( + self, clean_registry + ): + """Test that a declined shape still expands with no generic expander. + + Given: + Only the built-in DuckDB override registered, leaving + ``(GenericTarget, Intersects)`` unresolvable. + When: + Transpiling a FULL OUTER join with dialect='duckdb'. + Then: + It should fall back to the built-in naive overlap predicate rather + than calling ``None``. + """ + # Arrange + clean_registry.register(DuckDBTarget(), Intersects, expand_intersects_duckdb) + query = ( + "SELECT a.chrom, b.start FROM peaks a " + "FULL JOIN genes b ON a.interval INTERSECTS b.interval" + ) + + # Act + sql = transpile(query, tables=["peaks", "genes"], dialect="duckdb") + + # Assert + assert clean_registry.resolve(GenericTarget(), Intersects) is None + assert 'a."chrom" = b."chrom"' in sql + def test_registry_should_resolve_duckdb_intersects_to_iejoin_override(self): """Test that the built-in DuckDB IEJoin override is registered. diff --git a/tests/test_table.py b/tests/test_table.py new file mode 100644 index 0000000..9736871 --- /dev/null +++ b/tests/test_table.py @@ -0,0 +1,109 @@ +"""Tests for the Table genomic-table configuration.""" + +import dataclasses + +import pytest + +from giql import Table + + +class TestTable: + """Tests for Table.""" + + def test___post_init___should_raise_when_coordinate_system_is_unknown(self): + """Test that an unknown coordinate_system is rejected at construction. + + Given: + A coordinate_system outside the documented ``0based`` / ``1based`` + pair. + When: + A Table is constructed with it. + Then: + It should raise ``ValueError`` naming the offending value. + """ + # Arrange, act, & assert + with pytest.raises(ValueError, match="coordinate_system"): + Table("peaks", coordinate_system="2based") + + def test___post_init___should_raise_when_interval_type_is_unknown(self): + """Test that an unknown interval_type is rejected at construction. + + Given: + An interval_type outside the documented ``half_open`` / ``closed`` + pair. + When: + A Table is constructed with it. + Then: + It should raise ``ValueError`` naming the offending value. + """ + # Arrange, act, & assert + with pytest.raises(ValueError, match="interval_type"): + Table("peaks", interval_type="inclusive") + + def test_table_should_raise_when_a_field_is_assigned_after_construction(self): + """Test that a constructed Table cannot be mutated. + + Given: + A validly constructed Table. + When: + One of its fields is assigned. + Then: + It should raise ``FrozenInstanceError``. + + The validation above runs only in ``__post_init__``, so a mutable Table + would let a caller install a value the constructor rejects. That value is + not inert: ``coordinate_system`` selects the coordinate translation the + emitted SQL performs, so an unvalidated one produces a predicate matching + no coordinate system at all, with no error raised anywhere. + """ + # Arrange + table = Table("peaks") + + # Act & assert + with pytest.raises(dataclasses.FrozenInstanceError): + table.coordinate_system = "2based" + + def test_table_should_build_a_variant_through_replace(self): + """Test that dataclasses.replace re-validates the variant it builds. + + Given: + A validly constructed Table. + When: + ``dataclasses.replace`` derives a variant from it. + Then: + A valid field value should produce a new Table, and an invalid one + should raise -- ``replace`` re-runs ``__post_init__``, which is what + makes it the supported way to vary a frozen config. + """ + # Arrange + table = Table("peaks") + + # Act + variant = dataclasses.replace(table, coordinate_system="1based") + + # Assert + assert variant.coordinate_system == "1based" + assert table.coordinate_system == "0based" + with pytest.raises(ValueError, match="coordinate_system"): + dataclasses.replace(table, coordinate_system="2based") + + def test_table_should_be_hashable_and_value_equal(self): + """Test that two Tables with identical fields are interchangeable. + + Given: + Two separately constructed Tables carrying identical field values. + When: + They are compared and used as set members. + Then: + They should be equal and collapse to one set entry. + + A plain dataclass sets ``__hash__`` to None once it generates ``__eq__``, + so Table was unhashable before it was frozen. + """ + # Arrange + first = Table("peaks", chrom_col="contig") + second = Table("peaks", chrom_col="contig") + + # Act & assert + assert first == second + assert len({first, second}) == 1 diff --git a/tests/test_targets.py b/tests/test_targets.py index d269e76..6d785ae 100644 --- a/tests/test_targets.py +++ b/tests/test_targets.py @@ -296,6 +296,35 @@ def test_resolve_target_with_datafusion_returns_datafusion(): assert isinstance(target, DataFusionTarget) +@pytest.mark.parametrize( + "target", + [GenericTarget(), DuckDBTarget(), DataFusionTarget()], + ids=["generic", "duckdb", "datafusion"], +) +def test_resolve_target_with_target_instance_returns_it_unchanged(target): + """Test resolution of a Target instance passed in place of a name. + + Given: + A Target instance rather than a dialect name. + When: + resolve_target is called. + Then: + It should return that same instance. + + Registering a one-off Target under a name to make it selectable mutates the + process-global registry, and that registration outlives the call. Accepting + the object removes the global side effect from the one-off case. Note the + resulting asymmetry, which the unsupported-name test below pins from the + other side: GenericTarget is accepted as an instance but has no selectable + name, because None is the one public spelling for it. + """ + # Act + resolved = resolve_target(target) + + # Assert + assert resolved is target + + @pytest.mark.parametrize( "dialect", [ diff --git a/tests/test_transpile.py b/tests/test_transpile.py index 315742f..135f3d1 100644 --- a/tests/test_transpile.py +++ b/tests/test_transpile.py @@ -1,5 +1,7 @@ """Tests for the transpile() function.""" +import re + import pytest import giql @@ -117,6 +119,61 @@ def test_transpile_no_strand_column(self): assert "SELECT" in sql +class TestTranspileDialectArgument: + """Tests for how the dialect argument is admitted.""" + + def test_transpile_accepts_a_target_instance(self): + """ + GIVEN a Target instance passed as dialect instead of its name + WHEN transpiling + THEN should produce the same SQL as passing the name + + Target, Capabilities and the concrete targets are all exported from the + package root, so rejecting the very objects the module publishes made + the published type and the published entry point fail to compose. + """ + query = ( + "SELECT a.chrom, b.start FROM peaks a " + "JOIN genes b ON a.interval INTERSECTS b.interval" + ) + + by_name = transpile(query, tables=["peaks", "genes"], dialect="duckdb") + by_instance = transpile( + query, tables=["peaks", "genes"], dialect=giql.DuckDBTarget() + ) + + # Session-variable tokens are normalised out: what this pins is that the + # instance resolves to the same target as the name, not how the tokens + # are minted. + token = re.compile(r"__giql_iejoin_[0-9a-f]+") + assert token.sub("VAR", by_instance) == token.sub("VAR", by_name) + + +class TestTranspileTablesArgument: + """Tests for how the tables argument is admitted.""" + + def test_transpile_rejects_non_table_object(self): + """ + GIVEN an object in tables that is neither a str nor a Table + WHEN transpiling + THEN should raise TypeError naming the offending type + + The entry was previously duck-typed for a .name attribute, so an + arbitrary object reached pass 1 and failed there with an AttributeError + naming an internal column attribute -- which told the caller nothing + about which argument was wrong. + """ + + class NotATable: + name = "peaks" + + with pytest.raises(TypeError, match="NotATable"): + transpile( + "SELECT * FROM peaks WHERE interval INTERSECTS 'chr1:1000-2000'", + tables=[NotATable()], + ) + + class TestTranspileMultipleTables: """Tests for transpilation with multiple tables."""