[PERF] Extend selective bound selection to keep='all' for range joins

AGENTS.md

@@ -400,6 +400,12 @@ that looked like it might fade out below 30k rows instead held flat at
 ~1.8-1.9x all the way to 50M, which is the number that actually matters
 for deciding whether it's worth fixing.
 
+### Conditional-Join Performance Research
+
+Detailed guidance for `conditional_join` internals, behavioral invariants,
+performance research, the inequality-joins paper, benchmarks, and verification
+lives in `janitor/functions/_conditional_join/AGENTS.md`.
+
 ### Code Style Rules
 
 - **Line length**: 88 characters (ruff default)
@@ -580,6 +586,10 @@ pass, and removed from here to avoid restating the same rule twice.
 - **2026-08-21**: Added mandatory adversarial review before every PR
 - **2026-08-21**: Added PR/issue writing convention (ELI5 for non-obvious changes)
 - **2026-08-21**: Added convention to extend benchmarks to scale (10M-50M rows)
+- **2026-08-21**: Added the inequality-joins paper as a design reference for
+  current and future `conditional_join` performance work
+- **2026-08-21**: Moved detailed conditional-join guidance into the scoped
+  `_conditional_join/AGENTS.md`
 - **2026-08-21**: Cleanup pass - removed 5x-duplicated markdownlint rule, 3x
   -duplicated pixi/notebook rules, and Learned Patterns entries already
   integrated into main sections

CHANGELOG.md

@@ -1,6 +1,7 @@
 # Changelog
 
 ## [Unreleased]
+-   [ENH] Range joins now also apply #1659's selective bound selection for `keep='all'`, not just `keep='first'`/`'last'` - the pathology was unbounded rather than merely mild for `keep='all'` on this path, so it was worth the same fix; matched row content is unaffected, only row order can change (never guaranteed to begin with). - Issue #1666 @samukweku
 -   [ENH] Range joins (a `<`/`<=` predicate combined with a `>`/`>=` predicate) in `conditional_join` now pick the most selective candidate independently for each bound, when there are 2+ eligible predicates of either type and `keep` is `'first'` or `'last'` - fixing the same class of pathological slowdown from unfavorable predicate ordering as Issue #1641, for both `join_algorithm='default'` and `'regions'`. - Issue #1659 @samukweku
 -   [ENH] `conditional_join` now picks the most selective `<`/`<=`/`>`/`>=` predicate as its binary-search anchor (instead of the first one supplied) when `keep` is `'first'` or `'last'`, fixing pathological slowdowns from unfavorable predicate ordering; the choice is estimated from a fixed-size sample so the selection cost no longer scales with input size; `keep='all'` output is unaffected. - Issue #1641 @samukweku
 -   [BUG] Fix `conditional_join` crash for `keep='last'` joins with a single `<`/`<=` window and multiple non-equi conditions - a missing `counts` argument to the Rust `index_starts_only_keep_last` call. - Issue #1641 @samukweku

janitor/functions/_conditional_join/AGENTS.md

@@ -0,0 +1,103 @@
+# Conditional Join Agent Guidance
+
+This file extends the repository-level `AGENTS.md` for work under
+`janitor/functions/_conditional_join/`.
+
+## Behavioral Invariants
+
+- Preserve exact output values, row ordering, indexes, column structure,
+  dtypes, null semantics, and extension-array behavior.
+- Preserve aggregation, `return_matching_indices`, and
+  `include_join_positions` behavior when changing index-generation internals.
+- Treat `keep="all"` ordering as observable behavior. Changing the anchor or
+  sort key can change its row order even when the set of matches is unchanged.
+- For `keep="first"` and `keep="last"`, verify that any reordered search path
+  still reduces matches using original right-row positions rather than scan
+  order.
+- Do not assume caller indexes are unique or contiguous unless the public
+  dispatch path has normalized them before the relevant operation. Prefer
+  position-space internally when positions are the intended representation.
+- Preserve stable sorting and the distinctions between `<`, `<=`, `>`, and
+  `>=`, especially around duplicates and nulls.
+
+## Architecture Map
+
+- `conditional_join.py` performs public validation, normalizes frames, and
+  dispatches to the internal algorithms.
+- `_get_indices_non_equi.py` selects the maintained non-equality path.
+- `_le_ge_1_or_more.py` handles one or more same-direction inequality
+  predicates for the default algorithm.
+- `_dual_non_equi.py` implements the two-comparison region-number algorithm.
+- `_not_range_join_regions.py` dispatches same-direction predicates for
+  `join_algorithm="regions"`.
+- `_helpers.py` contains shared filtering and index-materialization logic;
+  changes there can affect multiple join modes.
+
+Follow dispatch and result materialization end to end before concluding that
+an internal ordering or index representation is unobservable.
+
+## Performance Research
+
+When investigating current or future `conditional_join` performance work,
+include the region-number algorithms from Dathan and Trausan-Matu,
+*Algorithms for Computing Inequality Joins* (DATA 2018), among the design
+options considered: <https://doi.org/10.5220/0006826803570364>.
+
+The paper's two-comparison algorithm underlies the existing `regions` path.
+Section 3.2 describes a separate multi-comparison strategy that computes
+regions for every predicate and dynamically chooses the driving field.
+Consider both the current implementation and the paper's fuller algorithm
+when researching:
+
+- predicate or anchor selection;
+- ordered field-role assignment in the asymmetric regions algorithm;
+- relation orientation when frame sizes differ;
+- candidate generation, filtering, and materialization;
+- range joins and joins with three or more inequality predicates.
+
+Treat the paper as an option to evaluate, not an automatic implementation
+mandate. Account for correlations between predicates: individual selectivity
+does not necessarily identify the best predicate pair.
+
+## Benchmark Expectations
+
+Performance work must compare equivalent joins with different predicate
+orders and include, where relevant:
+
+- `<`, `<=`, `>`, and `>=`;
+- `keep="all"`, `keep="first"`, and `keep="last"`;
+- two predicates and three or more predicates;
+- selective-first, broad-first, and similarly selective predicates;
+- sorted and unsorted inputs;
+- dense, sparse, zero-match, and full-match cases;
+- narrow and wide frames;
+- nullable and extension dtypes;
+- small inputs, where estimator overhead dominates;
+- large inputs, extending toward 10M-50M rows when memory and runtime permit.
+
+Report absolute timings as well as ratios. Measure already-optimal inputs to
+detect regressions introduced by the optimizer itself. For sampled or
+estimated selection, include skewed, correlated, periodic, and rare-tail data,
+and state the estimator's expected failure modes.
+
+## Verification
+
+Run Python only through `pixi`, as required by the repository-level guidance.
+The primary focused suite is:
+
+```bash
+pixi run pytest tests/functions/test_conditional_join.py -v
+```
+
+During iteration, narrower selections are acceptable, but run the complete
+focused file before completion. Add independent-oracle tests where practical;
+cross-join-and-filter is suitable for small correctness fixtures.
+
+Tests for an optimizer must assert its decision directly, not only final output
+invariance. Final output can remain identical even when the intended anchor,
+predicate pair, or algorithmic branch was never selected.
+
+Before treating a pull request as complete, follow the root requirement for a
+fresh-context adversarial review, with particular attention to ordering,
+nullable data, duplicate values, index representations, and performance on
+already-selective inputs.

janitor/functions/_conditional_join/_get_indices_non_equi.py

@@ -43,11 +43,17 @@ def _maybe_select_better_range_bounds(
     both range-join algorithms always process `ge_gt` first and `le_lt`
     second regardless of which candidate either one is).
 
-    Mutates and returns `mapping`. The matched row set is unaffected
-    either way (see #1641's invariance proof, which this reuses); only
-    performance and, for `keep='all'`, row order can change - so this is
-    only worth calling when there's an actual choice to make and
-    `keep in ('first', 'last')`, mirroring #1658's own original scoping.
+    Mutates and returns `mapping`. The matched row set and its values are
+    unaffected either way, for every `keep` mode (see #1641's invariance
+    proof, which this reuses). For `keep='first'`/`'last'`, output is
+    invariant entirely. For `keep='all'`, bound choice can still affect
+    output row order (which column `right` gets sorted by) - never row
+    content - see issue #1666, which extended this to `keep='all'` on
+    the same basis #1657 already established for the same-direction
+    path: that row order was never documented or guaranteed here to
+    begin with, and the pre-#1666 pathology for `keep='all'` on this
+    path was severe (unbounded, not the mild ~2-9x `keep='first'`/`'last'`
+    saw) - see #1666 for the numbers.
     """
     for bound_key, candidates_key in (
         ("le_lt", "le_lt_candidates"),
@@ -60,7 +66,11 @@ def _maybe_select_better_range_bounds(
         best = _select_range_bound(candidates, df, right)
         if best == current:
             continue
-        mapping["le_or_ge"] = [current, *[c for c in mapping["le_or_ge"] if c != best]]
+        # `best` is a le_or_ge member being promoted to `bound_key`;
+        # `current` demotes into its slot. Order within le_or_ge doesn't
+        # matter - it's always applied as an unordered set of independent
+        # post-filters - so an in-place swap is enough.
+        mapping["le_or_ge"][mapping["le_or_ge"].index(best)] = current
         mapping[bound_key] = best
     return mapping
 
@@ -102,7 +112,7 @@ def _get_indices(
             "left_index": empty_array,
             "right_index": empty_array,
         }
-    if mapping["is_range_join"] and keep in ("first", "last"):
+    if mapping["is_range_join"]:
         mapping = _maybe_select_better_range_bounds(mapping=mapping, df=df, right=right)
     if not mapping["is_range_join"]:
         if (len(mapping["le_or_ge"]) == 1) or (join_algorithm == "default"):

tests/functions/test_conditional_join.py

@@ -4913,22 +4913,97 @@ def test_range_join_ge_gt_order_invariant(join_algorithm, keep):
     assert_frame_equal(bad_order, good_order)
 
 
-def test_range_join_bound_selection_skipped_for_keep_all():
-    """`_maybe_select_better_range_bounds` must never run for `keep='all'`
-    - mirrors #1658's original scoping (row order, not content, is what's
-    at stake there); #1657-style extension to `keep='all'` is out of
-    scope for this fix."""
+def test_range_join_bound_selection_used_for_keep_all():
+    """`_maybe_select_better_range_bounds` must be invoked for `keep='all'`
+    too (issue #1666, extending #1659 the same way #1657 extended #1658
+    for the same-direction case) - bound choice never changes matched row
+    content, only (for `keep='all'`) the order those rows come back in,
+    so there's no correctness reason to leave `keep='all'` on the
+    unselective first-supplied-bound path."""
     df, right = _skewed_range_frames(seed=2, n=50)
     ge_gt = ("a", "lo", ">")
     narrow = ("a", "hi_narrow", "<")
     broad = ("a", "hi_broad", "<")
 
+    from janitor.functions._conditional_join import _get_indices_non_equi as m
+
     with mock.patch(
         "janitor.functions._conditional_join._get_indices_non_equi."
-        "_maybe_select_better_range_bounds"
+        "_maybe_select_better_range_bounds",
+        wraps=m._maybe_select_better_range_bounds,
     ) as patched:
         df.conditional_join(right, ge_gt, broad, narrow, keep="all", how="inner")
-        patched.assert_not_called()
+        patched.assert_called_once()
+
+
+@pytest.mark.parametrize("join_algorithm", ["default", "regions"])
+def test_range_join_le_lt_content_invariant_keep_all(join_algorithm):
+    """For `keep='all'`, the *set* of matched rows (and their values) must
+    not depend on which order the two `<`-type candidates are supplied
+    in, even though row order is no longer guaranteed to match between
+    orders (see #1666) - compare sorted, mirroring the pre-existing
+    `keep='all'` tests elsewhere in this file."""
+    df, right = _skewed_range_frames(seed=4)
+    ge_gt = ("a", "lo", ">")
+    narrow = ("a", "hi_narrow", "<")
+    broad = ("a", "hi_broad", "<")
+    columns = ["a", "lo", "hi_narrow", "hi_broad"]
+
+    bad_order = df.conditional_join(
+        right,
+        ge_gt,
+        broad,
+        narrow,
+        keep="all",
+        how="inner",
+        join_algorithm=join_algorithm,
+    ).sort_values(columns, ignore_index=True)
+    good_order = df.conditional_join(
+        right,
+        ge_gt,
+        narrow,
+        broad,
+        keep="all",
+        how="inner",
+        join_algorithm=join_algorithm,
+    ).sort_values(columns, ignore_index=True)
+    assert_frame_equal(bad_order, good_order)
+
+
+def test_range_join_row_order_can_differ_for_keep_all_on_tie():
+    """Documents the actual trade-off #1666 accepts, mirroring #1657 for
+    the same-direction case: for `keep='all'`, which candidate is picked
+    for `le_lt` is the secondary sort key `right` is ordered by whenever
+    `ge_gt`'s right column isn't already strictly sorted - so ties there
+    let bound choice affect row order. Verified empirically: this needs
+    an actual tie in the `ge_gt` column that also isn't already
+    monotonic (a constant `ge_gt` column, e.g., never triggers a real
+    sort at all, so never reproduces this) - once both hold, the two
+    argument orders produce the same content in a different order.
+    Content stays identical either way (see the content-invariance test
+    above); this test exists to make the trade-off visible, not to pin a
+    specific order."""
+    df = pd.DataFrame({"a": [0]})
+    right = pd.DataFrame(
+        {
+            "lo": [5, -100, -100, -100],
+            "hi_x": [999, 130, 110, 120],
+            "hi_y": [999, 101, 103, 102],
+        }
+    )
+    ge_gt = ("a", "lo", ">")
+    cond_x = ("a", "hi_x", "<")
+    cond_y = ("a", "hi_y", "<")
+
+    x_first = df.conditional_join(right, ge_gt, cond_x, cond_y, keep="all", how="inner")
+    y_first = df.conditional_join(right, ge_gt, cond_y, cond_x, keep="all", how="inner")
+
+    columns = list(x_first.columns)
+    assert_frame_equal(
+        x_first.sort_values(columns, ignore_index=True),
+        y_first.sort_values(columns, ignore_index=True),
+    )
+    assert not x_first.reset_index(drop=True).equals(y_first.reset_index(drop=True))
 
 
 def test_range_join_bound_selection_noop_with_single_candidate():