Skip to content

Commit a72bb22

Browse files
authored
fix: Fix MemorySegment::Compare to use big-endian byte-order comparison semantics (alibaba#298)
1 parent 7a2b6c4 commit a72bb22

3 files changed

Lines changed: 38 additions & 5 deletions

File tree

src/paimon/common/memory/memory_segment.cpp

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,8 @@ namespace paimon {
2323
int32_t MemorySegment::Compare(const MemorySegment& seg2, int32_t offset1, int32_t offset2,
2424
int32_t len) const {
2525
while (len >= 8) {
26-
// TODO(yonghao.fyh): support big endian, decide SmallEndian or BigEndian
27-
// long l1 = GetLongBigEndian(offset1);
28-
// long l2 = seg2.getLongBigEndian(offset2);
29-
uint64_t l1 = GetValue<int64_t>(offset1);
30-
uint64_t l2 = seg2.GetValue<int64_t>(offset2);
26+
uint64_t l1 = GetLongBigEndian(offset1);
27+
uint64_t l2 = seg2.GetLongBigEndian(offset2);
3128

3229
if (l1 != l2) {
3330
return (l1 < l2) ? -1 : 1;

src/paimon/common/memory/memory_segment.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
#include <memory>
2323
#include <type_traits>
2424

25+
#include "paimon/common/utils/math.h"
26+
#include "paimon/io/byte_order.h"
2527
#include "paimon/memory/bytes.h"
2628
#include "paimon/visibility.h"
2729

@@ -132,6 +134,14 @@ class PAIMON_EXPORT MemorySegment {
132134
std::memcpy(MutableData() + index, &value, sizeof(T));
133135
}
134136

137+
inline uint64_t GetLongBigEndian(int32_t index) const {
138+
auto value = GetValue<uint64_t>(index);
139+
if constexpr (SystemByteOrder() == ByteOrder::PAIMON_LITTLE_ENDIAN) {
140+
return EndianSwapValue(value);
141+
}
142+
return value;
143+
}
144+
135145
void CopyTo(int32_t offset, MemorySegment* target, int32_t target_offset,
136146
int32_t num_bytes) const {
137147
assert(offset >= 0);

src/paimon/common/memory/memory_segment_test.cpp

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,32 @@ TEST(MemorySegmentTest, TestCompare) {
158158
seg2.Put(i + 8, static_cast<char>(10));
159159
ASSERT_EQ(seg1.Compare(seg2, i, i, 7, 7), 0);
160160
ASSERT_LT(seg1.Compare(seg2, i, i, 9, 9), 0);
161+
162+
// Verify big-endian byte-order comparison semantics within a single 8-byte block.
163+
// On little-endian machines, naive native-endian uint64 comparison would give wrong results.
164+
MemorySegment seg3 = MemorySegment::AllocateHeapMemory(16, pool.get());
165+
MemorySegment seg4 = MemorySegment::AllocateHeapMemory(16, pool.get());
166+
Bytes zeros(16, pool.get());
167+
seg3.Put(0, zeros);
168+
seg4.Put(0, zeros);
169+
170+
// seg3: [0x00, 0x01, 0, 0, 0, 0, 0, 0] at offset 0
171+
// seg4: [0x01, 0x00, 0, 0, 0, 0, 0, 0] at offset 0
172+
// Lexicographic (byte-order) comparison: first byte 0x00 < 0x01, so seg3 < seg4.
173+
seg3.Put(1, static_cast<char>(0x01));
174+
seg4.Put(0, static_cast<char>(0x01));
175+
ASSERT_LT(seg3.Compare(seg4, 0, 0, 8), 0);
176+
ASSERT_GT(seg4.Compare(seg3, 0, 0, 8), 0);
177+
178+
// seg3: [0x01, 0x02, 0, 0, 0, 0, 0, 0]
179+
// seg4: [0x01, 0x01, 0, 0, 0, 0, 0, 0]
180+
// First bytes equal (0x01 == 0x01), second byte 0x02 > 0x01, so seg3 > seg4.
181+
seg3.Put(0, static_cast<char>(0x01));
182+
seg3.Put(1, static_cast<char>(0x02));
183+
seg4.Put(0, static_cast<char>(0x01));
184+
seg4.Put(1, static_cast<char>(0x01));
185+
ASSERT_GT(seg3.Compare(seg4, 0, 0, 8), 0);
186+
ASSERT_LT(seg4.Compare(seg3, 0, 0, 8), 0);
161187
}
162188

163189
TEST(MemorySegmentTest, TestCharAccess) {

0 commit comments

Comments
 (0)