Skip to content

[python] Honor sequence.field in primary-key writes - #10166

Merged
JingsongLi merged 4 commits into
apache:masterfrom
TheR1sing3un:codex/python-writer-sequence-field
Sep 25, 2026
Merged

JingsongLi merged 4 commits into
apache:masterfrom
TheR1sing3un:codex/python-writer-sequence-field

Conversation

@TheR1sing3un

@TheR1sing3un TheR1sing3un commented Sep 24, 2026 •

Copy link
Copy Markdown
Member

Purpose

A primary-key table with sequence.field=seq can keep a stale row when a newer business version arrives before an older version in the same write buffer. Writing (seq=100, value='high') followed by (seq=50, value='low') persists low, while committing the rows separately returns high.

Sort buffered rows by primary key, configured sequence fields and generated sequence number before folding equal keys. Use Arrow's columnar sort with temporary integer keys for floating sequence fields. Both Python read and write paths follow Java Float/Double comparison: NaNs compare equal above positive infinity, and negative zero precedes positive zero. Nulls stay first in both directions; ties fall back to the next sequence field and generated sequence number.

Validate sequence configuration and supported types before native backend selection and Python writer initialization, so invalid configurations cannot bypass checks or mutate dynamic bucket indexes.

Use the Python writer for floating or descending sequences and the Python reader for floating sequences, which the current native backend cannot order consistently. Supported native paths remain available.

Tests

  • Real-table tests cover batch/stream writers, Arrow batches, row writes, separate commits, buffer rolling, compound sequences, partitioned keys, partial-update null filling and typed sequences.
  • FLOAT/DOUBLE regressions cover NaN, infinities, signed zero, nulls, ties and both directions across write groupings. Dynamic bucket regressions cover builder/direct construction; native-selection regressions verify validation precedes the native factory even when the optional extension is absent. Of 50 review regressions, 46 fail before the fixes.
  • Native capability regressions verify the fallback boundaries and retain native selection for supported ascending integer sequences. Five of these six cases fail before the guards. Floating sequence end-to-end cases explicitly enable native read/write options.
  • Mixed-language CI now writes eight Python fixtures (FLOAT/DOUBLE × ascending/descending × same-batch/separate-commit) and checks them using the Java reader. Assertions verify both the winning payload and persisted sequence, covering NaN signs, signed zero, infinities, nulls and ties. Local verification passed all eight Python fixture tests and JavaPyE2ETest#testReadPythonFloatingSequence on Java 8, with Checkstyle, Spotless and Enforcer enabled. A test-only mutation bypassing the IEEE sort keys triggers Java assertion failures for NaN and signed-zero winners.
  • PyArrow 19.0.1 / NumPy 2.4.6: 183 sequence/native-bridge tests passed; 269 general write, merge-buffer, dispatch, aggregation and partial-update tests passed, plus 29 subtests. Another 168 sequence, native-read and read-builder integration cases passed after adding the native guards.
  • PyArrow 16.0.0 / NumPy 1.26.4: 214 sequence, native-bridge, merge-buffer and dispatch tests passed.
  • 14 integration cases requiring the optional Rust extension were skipped locally; full backend coverage is provided by Native CI.
  • Legacy Arrow compatibility: use NumPy's array protocol for both values and null masks, avoiding the unsupported ChunkedArray.to_numpy(zero_copy_only=...) keyword on Arrow 6/12. The existing eight fixture tests pass on Linux Python 3.6.15 / Arrow 6.0.1 / NumPy 1.19.5 and on Python 3.10 / Arrow 12.0.1 / NumPy 1.23.5; Java 8 reads all eight tables from each environment successfully. Final Maven checks run without fast-build. Also passed 129 Arrow 16 sequence/merge-buffer tests and 28 Arrow 19 / NumPy 2 floating regressions.
  • Changed-file flake8, Python 3.6 syntax parsing, license headers and git diff --check passed.

Comment thread paimon-python/pypaimon/write/writer/key_value_data_writer.py
Comment thread paimon-python/pypaimon/tests/test_sequence_field_read.py
Comment thread paimon-python/pypaimon/write/writer/key_value_data_writer.py Outdated
@TheR1sing3un
TheR1sing3un force-pushed the codex/python-writer-sequence-field branch from 8a6c994 to 2605de1 Compare September 25, 2026 03:12
@JingsongLi

Copy link
Copy Markdown
Contributor

Reviewed the current head as a data-correctness change, including the writer sort/fold path, read-side comparator, native dispatch boundaries, and the earlier review findings. There is clear end-to-end value: without this, buffering (seq=100, high) before (seq=50, low) can persist the wrong winner for a primary-key table. The current implementation orders buffered rows by PK, configured sequence fields, and generated sequence number; it uses explicit IEEE keys so NaNs and signed zero follow Java ordering. Invalid sequence configurations are checked before native selection and before dynamic-bucket row-key extraction. Floating native reads and floating/descending native writes fall back to the Python implementations where the native comparator is not equivalent.

Local verification on this head: 183 focused sequence/native tests passed (14 optional-Rust cases skipped locally); 156 adjacent write-buffer, partial-update, aggregation, and table-write tests passed, plus 6 subtests. git diff --check passed. CI is green for Native CI and Python 3.6, 3.7, 3.10, 3.11, 3.12, and 3.13. The new real-table tests cover batch/stream and multiple write groupings, NaN/infinity/signed-zero/null ordering, compound fields, buffer flushes, and early invalid-configuration rejection. The three previously reported issues appear addressed at this head.

I found no blocking regression. A Python-write/Java-read interoperability smoke test would add confidence in the shared-table contract, especially for floating sequences, but the current writer ordering and comparator agree with the Java path I inspected.

@JingsongLi
JingsongLi merged commit 66e2530 into apache:master Sep 25, 2026
14 checks passed
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.

3 participants