wangzhigang1999 commented on code in PR #10493:
URL: https://github.com/apache/paimon/pull/10493#discussion_r4256420380
##########
paimon-python/pypaimon/read/reader/sort_merge_reader.py:
##########
@@ -257,19 +245,10 @@ def comparator(row1: InternalRow, row2: InternalRow) ->
int:
raise ValueError(f"Unsupported {fields[idx].type} comparison")
if floating_flags[pos]:
- # Java Float/Double.compare: all NaNs tie above +inf, and
- # -0.0 precedes +0.0. Primary-key equality is unchanged.
- if math.isnan(val1):
- if not math.isnan(val2):
- return sign
- continue
- if math.isnan(val2):
- return -sign
- if val1 == val2 == 0.0:
- val1 = math.copysign(1.0, val1)
- val2 = math.copysign(1.0, val2)
-
- if val1 < val2:
+ result = compare_values(val1, val2)
Review Comment:
I realize your P1 explicitly requires a runnable, reviewable migration tool.
I'd like to revisit that requirement rather than silently remove the tool.
Our prototype already excluded schema evolution, deletion vectors, row
tracking, and complex types. Testing also showed that rebuilding live rows can
drop tombstones and change late-event behavior. Shipping it, even as a
reference, could give users false confidence.
I propose keeping the ordering fix, a metadata-only gate that rejects
unacknowledged floating-PK tables, and documented upgrade acceptance criteria,
with users choosing and validating their migration procedure. Java reads/writes
remain unaffected; verified Java-only history can be acknowledged without
rewriting.
Would you accept that scope for this PR, with a supported migration tool
tracked separately if needed? I wouldn't consider the P1 resolved without your
agreement.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
