Repository navigation
Keep argument column order when a join swaps its sides - #86
Merged
Merged
Conversation
inner-join and outer-join stream the larger input and group the smaller one, swapping a and b when b's file is bigger. The swap leaked into the output: without :select, the second dataframe's columns (2_*) came first, in the header, the data, and preview. JoinedDataFrame gains a swapped flag, set only by the swapping branches of inner-join and outer-join: - getColNames presents the caller's order (record b first when swapped), so headers, preview and :select name resolution all follow it. - compute translates the user-facing select indices to internal ones (rows are always record-a columns then record-b columns); the existing write-index machinery then reorders the emitted values, so the join workers are untouched. - preview maps each side's old names onto the right half of the new ones. right-join keeps its existing swapped output order (its swap is semantic, not a size heuristic), and rolling joins never swap. Tests written first and seen failing on the 2_*-first order: a fast api-test for getColNames and preview, and an Onyx-backed regression test over disk and in-memory paths, including a :select on a swapped join. Existing fixtures are unaffected: the join tests' first input is the larger file, and the swapped-order fixtures all use :select.
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Batch 3 of the agreed plan.
inner-joinandouter-joinkeep the swap-smaller-side heuristic (stream the larger input, group the smaller), but the swap no longer leaks into the output: without:select, headers, data, and preview now present the caller's column order (1_*first) instead of the second dataframe's (2_*first).Mechanism:
JoinedDataFramegains aswappedflag, set only by the swapping branches ofinner-join/outer-join.getColNamespresents the caller's order, so:selectname resolution and validation follow it for free.computetranslates the user-facing select indices to internal ones (internal rows are always record-a then record-b columns) and feeds those to the existingwrite-indexcomputation, which already reorders emitted values into select order — the join and outer-join workers are completely untouched.Out of scope, unchanged:
right-join's swap is semantic rather than a size heuristic, so its output order stays as is; rolling joins never swap.Tests written first and seen failing on the
2_*-first order:getColNamesandpreviewon a swapped inner/outer join.:selecton a swapped join.No fixture changes needed: the existing join tests' first input is the larger file, and the swapped-order fixtures (1-6, 1-12) all use explicit
:select. The plan's fixture note turned out unnecessary — the regression test writes its own inputs and asserts inline, like the #84 tests.Suite: 52 tests / 198 assertions, green locally on JDK 25. No version bump; rides with #83–#85 in a later 2.0.6.