You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "TheR1sing3un (via GitHub)" <gi...@apache.org> on 2026/09/24 16:25:48 UTC
[PR] [python] Honor sequence.field in primary-key writes [paimon]
TheR1sing3un opened a new pull request, #10166:
URL: https://github.com/apache/paimon/pull/10166
### 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')` currently persists `low`; committing the rows separately returns `high` because the read-side merge already honors the sequence field.
Sort buffered rows by primary key, configured sequence fields and generated sequence number before folding equal keys. Keep the columnar Arrow sort, nulls-first ordering in both directions, and generated sequence numbers as the tie breaker. Validate sequence configuration and supported field types before buffering.
### Tests
- 202 focused writer, merge-buffer, dispatch, aggregation, partial-update and sequence-field tests plus 6 subtests passed on Python 3.11 / PyArrow 19.0.1.
- 42 new real-table cases cover batch and stream writers; one batch, separate Arrow batches, row writes and separate commits; ascending/descending order, nulls, ties, compound fields, partitioned keys, partial-update null filling, buffer rolling, projection and typed sequence fields. Replacing the sort with the original implementation causes 33 failures; 9 unaffected cases pass.
- Combined with #10162 and unsupported-aggregation write validation: 135 focused tests passed.
- Changed-file flake8, Python 3.6 syntax parsing, license headers and `git diff --check` passed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [python] Honor sequence.field in primary-key writes [paimon]
Posted by "TheR1sing3un (via GitHub)" <gi...@apache.org>.
TheR1sing3un commented on code in PR #10166:
URL: https://github.com/apache/paimon/pull/10166#discussion_r4101818355
##########
paimon-python/pypaimon/write/writer/key_value_data_writer.py:
##########
@@ -286,6 +295,11 @@ def _sort_by_primary_key(
# Table, so this serves both the per-batch entry path (legacy)
# and the buffer-wide sort path (used by ``_flush_all``).
sort_keys = [(key, 'ascending') for key in self.trimmed_primary_keys]
+ sequence_fields = self.options.sequence_field()
Review Comment:
It has been fixed. Thank you for your suggestion！
##########
paimon-python/pypaimon/tests/test_sequence_field_read.py:
##########
@@ -301,7 +302,8 @@ def test_missing_sequence_field_rejected(self):
"""
table = self._create_pk_table(
Review Comment:
It has been fixed. Thank you for your suggestion！
##########
paimon-python/pypaimon/write/writer/key_value_data_writer.py:
##########
@@ -45,6 +45,15 @@ def __init__(self, table, partition, bucket, max_seq_number,
changelog_producer=ChangelogProducer.NONE):
super().__init__(table, partition, bucket, max_seq_number,
options, write_cols, changelog_producer)
+ if self.options.sequence_field():
Review Comment:
It has been fixed. Thank you for your suggestion！
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [python] Honor sequence.field in primary-key writes [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi merged PR #10166:
URL: https://github.com/apache/paimon/pull/10166
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [python] Honor sequence.field in primary-key writes [paimon]
Posted by "Akash3121 (via GitHub)" <gi...@apache.org>.
Akash3121 commented on code in PR #10166:
URL: https://github.com/apache/paimon/pull/10166#discussion_r4097389842
##########
paimon-python/pypaimon/tests/test_sequence_field_read.py:
##########
@@ -301,7 +302,8 @@ def test_missing_sequence_field_rejected(self):
"""
table = self._create_pk_table(
Review Comment:
This expectation currently fails under the required Native CI configuration because validation was added only to `KeyValueDataWriter`, which is never constructed when `WriteBuilder` selects the native deduplicate writer. Native CI shows the same bypass for missing, duplicate and empty sequence fields and for cross-partition updates: all four writes complete without the expected `ValueError`. Please perform table-level sequence validation before native/Python backend selection (or add equivalent native validation); marking these tests as Python-only would hide a real behavior difference.
##########
paimon-python/pypaimon/write/writer/key_value_data_writer.py:
##########
@@ -45,6 +45,15 @@ def __init__(self, table, partition, bucket, max_seq_number,
changelog_producer=ChangelogProducer.NONE):
super().__init__(table, partition, bucket, max_seq_number,
options, write_cols, changelog_producer)
+ if self.options.sequence_field():
Review Comment:
This constructor is reached only after `TableWrite.write_arrow_batch` calls `row_key_extractor.extract_partition_bucket_groups`. With `bucket=-1`, that call records new mappings in `DynamicBucketIndexMaintainer` before this validation can reject a missing/duplicate/unsupported sequence field. If the caller catches the error and invokes `prepare_commit`, it can receive index-only commit messages and create HASH index files for rows that were never accepted. Please move validation ahead of row-key extraction and add a dynamic-bucket regression asserting that a rejected write leaves `prepare_commit` empty and creates no index files.
##########
paimon-python/pypaimon/write/writer/key_value_data_writer.py:
##########
@@ -286,6 +295,11 @@ def _sort_by_primary_key(
# Table, so this serves both the per-batch entry path (legacy)
# and the buffer-wide sort path (used by ``_flush_all``).
sort_keys = [(key, 'ascending') for key in self.trimmed_primary_keys]
+ sequence_fields = self.options.sequence_field()
Review Comment:
This does not match the read-side comparator for FLOAT/DOUBLE NaNs, so the same rows can still produce different winners depending on grouping. `null_placement='at_start'` also places NaNs before finite values in Arrow. For rows written as `(seq=1.0, value='finite-first')` followed by `(seq=NaN, value='nan-last')`, one buffered write sorts NaN first and therefore keeps finite-first . Across separate commits, `_row_field_comparator` treats NaN and `1.0` as equal because both `<` and `>` are false, then the generated/file sequence number breaks the tie, so the later `nan-last` row wins. Please define one NaN ordering shared by the writer and `builtin_seq_comparator` (and aligned with Java), and add batch/chunk/commit regressions for NaN in both sort directions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [python] Honor sequence.field in primary-key writes [paimon]
Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on PR #10166:
URL: https://github.com/apache/paimon/pull/10166#issuecomment-5831882161
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@paimon.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org