Skip to content

fix: Fix MemorySegment::Compare to use big-endian byte-order comparison semantics#298

Merged
lxy-9602 merged 6 commits into
alibaba:mainfrom
lxy-9602:fix-memseg-cmp
May 25, 2026
Merged

fix: Fix MemorySegment::Compare to use big-endian byte-order comparison semantics#298
lxy-9602 merged 6 commits into
alibaba:mainfrom
lxy-9602:fix-memseg-cmp

Conversation

@lxy-9602
Copy link
Copy Markdown
Collaborator

Purpose

No Linked issue.

Problem

MemorySegment::Compare reads 8 bytes at a time using GetValue<int64_t>() which returns the value in native endianness. On little-endian platforms (x86/ARM), this breaks lexicographic (byte-order) comparison semantics — the most significant byte in memory becomes the least significant byte in the integer, causing incorrect comparison results.

For example, comparing [0x00, 0x01, 0, 0, 0, 0, 0, 0] vs [0x01, 0x00, 0, 0, 0, 0, 0, 0]:

  • Expected (lexicographic): first byte 0x00 < 0x01, result should be negative
  • Before fix (little-endian native): reads as 0x0100 > 0x0001, result was positive ❌

This is consistent with the existing TODO in the code and aligned with the Java implementation which uses getLongBigEndian().

Fix

  • Added GetLongBigEndian() method to MemorySegment that reads 8 bytes and converts to big-endian order using the existing EndianSwapValue() utility from math.h
  • Updated Compare() to call GetLongBigEndian() instead of raw GetValue<int64_t>()

Tests

MemorySegmentTest.TestCompare

API and Format

Documentation

Generative AI tooling

Generated-by: Claude-4.7-Opus

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

Comment thread src/paimon/common/memory/memory_segment.h
Comment thread src/paimon/common/memory/memory_segment.h
Comment thread src/paimon/common/memory/memory_segment_test.cpp
lxy-9602 and others added 4 commits May 25, 2026 18:15
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@lxy-9602 lxy-9602 merged commit a72bb22 into alibaba:main May 25, 2026
9 checks passed
lxy-9602 added a commit to lxy-9602/paimon-cpp that referenced this pull request May 28, 2026
lxy-9602 added a commit that referenced this pull request May 28, 2026
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