Skip to content

fix: stable sort in PolarsBuilderVisitor.visit_order_by - #21

Merged
adsharma merged 1 commit into
mainfrom
fix/polars-stable-sort
Oct 20, 2026
Merged

adsharma merged 1 commit into
mainfrom
fix/polars-stable-sort

Conversation

@adsharma

Copy link
Copy Markdown
Owner

What was broken

CI (build (ubuntu-latest, 3.12) on PR #20, also reproducible on main) fails in tests/test_polars.py::PolarsTests::test_project:

AssertionError: DataFrames are different (value mismatch for column ":id")
[left]:  [1, 4, 2]   expected
[right]: [1, 4, 3]   actual (varies run to run; also seen [1, 4, 9])

This failure is pre-existing and unrelated to PR #20 — it fails identically on main (a8cdb82).

Root cause

Nondeterministic tie order in the Polars sort, via three compounding facts:

  1. Test data has ties by construction. User.get() assigns age=random.choice([16,17,18]). With the test's random.seed(100), ages are {1:16, 2:17, 3:17, 4:16, 5:18, 6:17, 7:18, 8:17, 9:17}, so where(age >= 16) keeps all 9 rows and order_by(age) has a 5-way tie at age 17. take(3) then picks [1, 4, <first 17-year-old in sort output>].
  2. The builder requests an unstable sort. visit_order_by applied LazyFrame.sort(field), whose defaults are maintain_order=False, multithreaded=True — equal elements may come out in any order.
  3. Proven flaky, not version-specific. Three identical local runs of the same query returned [1, 4, 2], [1, 4, 3], [1, 4, 2]. It passed on main back on Oct 5 purely by scheduling luck; polars is unpinned (>= 0.12.0), so any version/threading change reshuffles the ties.

Change

fquery/polars_builder.py: generalize the deferred-op stack entries from (func, params) to (func, params, kwargs) and pass maintain_order=True for sorts, so ties keep input order — matching the in-memory executor's behavior.

Verification

  • tests/test_polars.py: 6/6 consecutive runs pass (previously failed intermittently).
  • Full suite: 59 passed (baseline on main: 58 passed + the 1 polars failure).
  • black + flake8 clean on the touched file.

Polars LazyFrame.sort defaults to maintain_order=False, so rows with
equal sort keys come out in thread-scheduling order. Combined with the
randomized ages in tests/mock_user.py, order_by(age).take(3) picked a
different third row on different runs (e.g. [1,4,2] vs [1,4,3]), making
tests/test_polars.py::PolarsTests::test_project flaky in CI.

Generalize the op stack entries to (func, params, kwargs) and pass
maintain_order=True for sorts so ties keep input order, matching the
in-memory executor.
@adsharma
adsharma merged commit 27a36ef into main Oct 20, 2026
2 checks passed
@adsharma
adsharma deleted the fix/polars-stable-sort branch October 20, 2026 00:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant