From 0d1f83807092ae243595c453c3fb6e4fd1c0d261 Mon Sep 17 00:00:00 2001 From: Conrad Date: Tue, 1 Sep 2026 15:01:58 -0400 Subject: [PATCH 1/5] fix: Make Table immutable so its validation cannot be bypassed Table validated coordinate_system and interval_type in __post_init__ but was a plain dataclass, so assigning either field afterwards installed a value the constructor rejects. The value is not inert: it selects the coordinate translation the emitted SQL performs. Setting it to a third string produced an ON predicate that is neither the 0-based form nor the 1-based one but a half-shifted hybrid matching no coordinate system, and transpile raised nothing. Freezing makes __post_init__ the single way in, so validation holds for the object's whole lifetime rather than only at construction. Callers vary a config with dataclasses.replace, which re-runs that validation. Immutability also restores __hash__, which the generated __eq__ had set to None. Separately, _build_tables duck-typed every non-str entry for a .name attribute. An arbitrary object therefore reached pass 1 and failed there with an AttributeError naming an internal column attribute, telling the caller nothing about which argument was wrong. It now rejects the entry with a TypeError naming the offending type. Adds tests for the two field validations, which had none. --- src/giql/table.py | 10 +++- src/giql/transpile.py | 13 ++++- tests/test_table.py | 109 ++++++++++++++++++++++++++++++++++++++++ tests/test_transpile.py | 25 +++++++++ 4 files changed, 155 insertions(+), 2 deletions(-) create mode 100644 tests/test_table.py 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/transpile.py b/src/giql/transpile.py index 94e85f0..4927481 100644 --- a/src/giql/transpile.py +++ b/src/giql/transpile.py @@ -115,6 +115,9 @@ def transpile( 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 -------- @@ -236,8 +239,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/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_transpile.py b/tests/test_transpile.py index 315742f..dcdc5d1 100644 --- a/tests/test_transpile.py +++ b/tests/test_transpile.py @@ -117,6 +117,31 @@ def test_transpile_no_strand_column(self): assert "SELECT" in sql +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.""" From 5280fcdfc7c2858fc86bc5aa00243341c9743d58 Mon Sep 17 00:00:00 2001 From: Conrad Date: Tue, 1 Sep 2026 15:02:24 -0400 Subject: [PATCH 2/5] feat: Accept a Target instance as the dialect argument Target, Capabilities, GenericTarget, DuckDBTarget and DataFusionTarget are all exported from the package root and autodocumented, yet the only public function that consumes a target rejected every one of them and reported the object's repr as though it were a misspelled name. Selecting a target by name stays the documented default and is the right seam for the plugin-distribution case, where a package ships a target and users select it without importing it. What that does not cover is the one-off: making a bespoke Target selectable meant mutating the process-global registry, and the registration outlived the call. The object path removes that side effect and is purely additive. GenericTarget is now accepted as an instance while the name "generic" still raises. That asymmetry is deliberate: None remains the one public spelling for the generic target, but an instance is unambiguous. Also drops the three transpile overloads. All three returned str and the widest admitted a bare str, so the set collapsed to the implementation signature and taught a type checker nothing. Their stated purpose was editor completion of the built-in dialect names, which the DialectName literal alias in the signature preserves without three public typing artifacts. --- src/giql/targets.py | 20 +++++++++++++++++--- src/giql/transpile.py | 37 +++---------------------------------- tests/test_targets.py | 29 +++++++++++++++++++++++++++++ tests/test_transpile.py | 32 ++++++++++++++++++++++++++++++++ 4 files changed, 81 insertions(+), 37 deletions(-) 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 4927481..2d12de3 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. 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 dcdc5d1..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,36 @@ 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.""" From bb253ebb4e931b91df75dfb7bc860b8bf6b9ecbc Mon Sep 17 00:00:00 2001 From: Conrad Date: Tue, 1 Sep 2026 15:03:31 -0400 Subject: [PATCH 3/5] feat: Decompose LEFT and RIGHT INTERSECTS joins into two IEJoin halves DuckDB's IE_JOIN is INNER-only, so a LEFT or RIGHT outer join carrying a column-to-column INTERSECTS fell through to the naive predicate: a hash join on chrom with the position inequalities as a residual filter, which is quadratic when the chromosome key has low cardinality. The query is now rewritten as a UNION ALL of an INNER half for the matched pairs, a NOT EXISTS half for the preserved side's unmatched rows, and a third branch for its NULL-chromosome rows. The first two reach the fast operator; the third is a filtered scan, and it is load-bearing rather than defensive, because both partitions come from SELECT DISTINCT chrom where a NULL renders as a NULL literal that string_agg skips. RIGHT is served by swapping the FROM and joined tables so one LEFT-shaped path covers both. Shapes the rewrite cannot express decline as one unit. Two properties of the emission decide whether it is worth taking, and both are settled by execution rather than by plan inspection, which reports IE_JOIN either way. The first is contig cardinality. One UNION ALL branch is emitted per distinct chromosome, so cost tracks that count while the plain predicate's does not: measured at 262,144 rows per side, the partitioned form runs 0.79s against 4.53s naive at 24 contigs and 57.2s against 0.11s at 3,000. The partition's cardinality is a property of the data, so the choice is made at execution time by a CASE over the partition's own row count, above which the same query binds with the chromosome equality inlined instead. The unmatched half also partitions on the chromosome INTERSECT rather than the preserved side's distinct chromosomes, carrying its left-only chromosomes in one non-partitioned branch: those rows cannot match, so a branch apiece scans both tables to prove an emptiness the partition already knows. The second is session state. Each half declares a DuckDB session variable, and the emitted script cannot release them because the final statement has to be the SELECT. Names are therefore a digest of the variable's own rendered value, which bounds a session's variable set by the number of distinct query shapes rather than the number of calls; naming them per call retained 26 to 84 MB after 50 to 200 queries. The naive-predicate fallback resolves through the registry rather than calling the built-in directly, so a user expander registered on (GenericTarget, Intersects) reaches the shapes this target declines instead of applying under dialect=None alone. --- src/giql/expanders/_per_chrom.py | 99 +- src/giql/expanders/intersects.py | 11 +- src/giql/expanders/intersects_duckdb.py | 961 +++++++++---- tests/test_duckdb_iejoin.py | 1681 ++++++++++++++++++++++- tests/test_expander.py | 64 + 5 files changed, 2539 insertions(+), 277 deletions(-) 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/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. From 450f3a90f7305100725765d07fcc37b0cf486e1f Mon Sep 17 00:00:00 2001 From: Conrad Date: Tue, 1 Sep 2026 15:05:50 -0400 Subject: [PATCH 4/5] test: Mark the bedtools integration modules and pin the duckdb plan The bedtools lane declared its integration marker in conftest.py, where pytest does not honour pytestmark, leaving all of its tests unmarked. Both documented selection commands therefore inverted: running with the integration marker skipped the whole lane, and running without it pulled in the lane and its bedtools and pybedtools dependencies. CI was unaffected because it runs the suite with no marker filter, so nothing surfaced it. Each module now declares the marker itself, matching the datafusion lane's working convention. The cross-target oracle asserted only that the three targets agree, which it would continue to do if duckdb silently stopped decomposing. It now also pins which plan duckdb took, through a target-to-SQL map the oracle fixture exposes. Adds bedtools oracles for the RIGHT outer join, expressed as the left outer join with the operands swapped, and for duplicate input rows, the multiplicity axis a UNION ALL rewrite is most likely to break and which the property lane could not reach because it draws from unique inputs. --- tests/integration/bedtools/test_cluster.py | 4 + tests/integration/bedtools/test_contains.py | 4 + tests/integration/bedtools/test_distance.py | 2 + tests/integration/bedtools/test_intersect.py | 186 ++++++++++++++++++ .../bedtools/test_intersect_property.py | 60 +++++- tests/integration/bedtools/test_merge.py | 4 + tests/integration/bedtools/test_nearest.py | 4 + .../bedtools/test_nearest_property.py | 2 + .../integration/bedtools/test_strand_aware.py | 4 + tests/integration/bedtools/test_within.py | 4 + tests/integration/conftest.py | 7 +- .../datafusion/test_cross_target_oracle.py | 23 ++- 12 files changed, 295 insertions(+), 9 deletions(-) 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.""" From 0d05e4935c669916f74ad077bba14768d8edadf0 Mon Sep 17 00:00:00 2001 From: Conrad Date: Tue, 1 Sep 2026 15:06:05 -0400 Subject: [PATCH 5/5] docs: Document the decomposition, the ceiling and the dialect contract The dialect parameter promised that an unqualified projection raises at transpile time. That holds for the INNER, SEMI and ANTI shapes and not for the outer joins, which decline silently to the naive predicate instead, so one stated rule covered two behaviours with no way for a caller to tell which applied. The same paragraph carried an inline list of declined shapes that had drifted behind the guide it points at, so the list is gone and the pointer stays: a second copy of an enumeration only drifts again. Returns said the result was a SQL query. Under duckdb an accelerated join returns a multi-statement script, and a driver that splits statements or forwards only the last drops the variable the SELECT reads and yields empty results. The partition-count ceiling decides at execution time whether a query takes the per-chromosome form at all, and it is the difference between winning and losing by orders of magnitude on a scaffold-level assembly. The performance guide now carries the measured crossover and the reasoning behind the bound, rather than generalising from a single low-contig measurement to a claim of large speedups at scale. Two further claims are restated from measurements rather than intuition. The NULL-chromosome branch, described as costing a linear scan of the preserved table, is pruned by null statistics to a quarter of a percent of runtime on a base table. The per-chromosome LEFT JOIN comparison asserted an inflection near 1e5 rows that the recorded figures do not support; it now states the two points that were actually measured. Also documents what the session-variable token is, now that it addresses the variable's content rather than being random, and gives transform_to_sql its own contract in place of a description of its sibling: the script shape, the ValueError it raises, and that the query it is handed is never mutated. --- README.md | 2 +- docs/dialect/spatial-operators.rst | 2 +- docs/recipes/bedtools-migration.rst | 9 ++ docs/transpilation/performance.rst | 165 +++++++++++++++++++++------- src/giql/transpile.py | 71 ++++++++---- 5 files changed, 184 insertions(+), 65 deletions(-) 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/transpile.py b/src/giql/transpile.py index 2d12de3..40b80ee 100644 --- a/src/giql/transpile.py +++ b/src/giql/transpile.py @@ -43,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 @@ -77,7 +94,16 @@ 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 ------ @@ -121,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 "