Skip to content

Commit b9ec9a5

Browse files
committed
Merge branch 'main' into avro-reader-implementation
2 parents 2a2cced + 6f33763 commit b9ec9a5

File tree

55 files changed

+3369
-588
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

55 files changed

+3369
-588
lines changed

.github/workflows/integration.yml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -165,8 +165,9 @@ jobs:
165165
- name: Run Rust tests
166166
run: |
167167
source venv/bin/activate
168-
cargo test -p arrow-pyarrow
169-
- name: Run tests
168+
cd arrow-pyarrow-testing
169+
cargo test
170+
- name: Run Python tests
170171
run: |
171172
source venv/bin/activate
172173
cd arrow-pyarrow-integration-testing

.github/workflows/parquet.yml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,13 @@ jobs:
113113
run: cargo check -p parquet --all-targets --no-default-features --features json
114114
- name: Check compilation --no-default-features --features encryption --features async
115115
run: cargo check -p parquet --no-default-features --features encryption --features async
116+
- name: Check compilation --no-default-features --features flate2, this is expected to fail
117+
run: if `cargo check -p parquet --no-default-features --features flate2 2>/dev/null`; then false; else true; fi
118+
- name: Check compilation --no-default-features --features flate2 --features flate2-rust_backened
119+
run: cargo check -p parquet --no-default-features --features flate2 --features flate2-rust_backened
120+
- name: Check compilation --no-default-features --features flate2 --features flate2-zlib-rs
121+
run: cargo check -p parquet --no-default-features --features flate2 --features flate2-zlib-rs
122+
116123

117124
# test the parquet crate builds against wasm32 in stable rust
118125
wasm32-build:

.github/workflows/rust.yml

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ jobs:
5252
# do not produce debug symbols to keep memory usage down
5353
export RUSTFLAGS="-C debuginfo=0"
5454
# PyArrow tests happen in integration.yml.
55-
cargo test --workspace --exclude arrow-pyarrow
55+
cargo test --workspace
5656
5757
5858
# Check workspace wide compile and test with default features for
@@ -84,8 +84,7 @@ jobs:
8484
# do not produce debug symbols to keep memory usage down
8585
export RUSTFLAGS="-C debuginfo=0"
8686
export PATH=$PATH:/d/protoc/bin
87-
# PyArrow tests happen in integration.yml.
88-
cargo test --workspace --exclude arrow-pyarrow
87+
cargo test --workspace
8988
9089
9190
# Run cargo fmt for all crates

Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,9 @@ members = [
5555
resolver = "2"
5656

5757
exclude = [
58+
# arrow-pyarrow-testing is excluded because it requires a Python interpreter with the pyarrow package installed,
59+
# which makes running `cargo test --all` fail if the appropriate Python environment is not set up.
60+
"arrow-pyarrow-testing",
5861
# arrow-pyarrow-integration-testing is excluded because it requires different compilation flags, thereby
5962
# significantly changing how it is compiled within the workspace, causing the whole workspace to be compiled from
6063
# scratch this way, this is a stand-alone package that compiles independently of the others.

arrow-array/benches/view_types.rs

Lines changed: 59 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,17 +27,74 @@ fn gen_view_array(size: usize) -> StringViewArray {
2727
}))
2828
}
2929

30+
fn gen_view_array_without_nulls(size: usize) -> StringViewArray {
31+
StringViewArray::from_iter((0..size).map(|v| {
32+
let s = match v % 3 {
33+
0 => "small".to_string(), // < 12 bytes
34+
1 => "larger than 12 bytes array".to_string(), // >12 bytes
35+
2 => "x".repeat(300), // 300 bytes (>256)
36+
_ => unreachable!(),
37+
};
38+
Some(s)
39+
}))
40+
}
41+
3042
fn criterion_benchmark(c: &mut Criterion) {
3143
let array = gen_view_array(100_000);
3244

33-
c.bench_function("gc view types all", |b| {
45+
c.bench_function("gc view types all[100000]", |b| {
3446
b.iter(|| {
3547
black_box(array.gc());
3648
});
3749
});
3850

3951
let sliced = array.slice(0, 100_000 / 2);
40-
c.bench_function("gc view types slice half", |b| {
52+
c.bench_function("gc view types slice half[100000]", |b| {
53+
b.iter(|| {
54+
black_box(sliced.gc());
55+
});
56+
});
57+
58+
let array = gen_view_array_without_nulls(100_000);
59+
60+
c.bench_function("gc view types all without nulls[100000]", |b| {
61+
b.iter(|| {
62+
black_box(array.gc());
63+
});
64+
});
65+
66+
let sliced = array.slice(0, 100_000 / 2);
67+
c.bench_function("gc view types slice half without nulls[100000]", |b| {
68+
b.iter(|| {
69+
black_box(sliced.gc());
70+
});
71+
});
72+
73+
let array = gen_view_array(8000);
74+
75+
c.bench_function("gc view types all[8000]", |b| {
76+
b.iter(|| {
77+
black_box(array.gc());
78+
});
79+
});
80+
81+
let sliced = array.slice(0, 8000 / 2);
82+
c.bench_function("gc view types slice half[8000]", |b| {
83+
b.iter(|| {
84+
black_box(sliced.gc());
85+
});
86+
});
87+
88+
let array = gen_view_array_without_nulls(8000);
89+
90+
c.bench_function("gc view types all without nulls[8000]", |b| {
91+
b.iter(|| {
92+
black_box(array.gc());
93+
});
94+
});
95+
96+
let sliced = array.slice(0, 8000 / 2);
97+
c.bench_function("gc view types slice half without nulls[8000]", |b| {
4198
b.iter(|| {
4299
black_box(sliced.gc());
43100
});

arrow-array/src/array/byte_view_array.rs

Lines changed: 120 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -583,7 +583,7 @@ impl<T: ByteViewType + ?Sized> GenericByteViewArray<T> {
583583
/// little-endian `u128` representation, converts them to big-endian ordering, and packs them
584584
/// into a single `u128` value suitable for fast, branchless comparisons.
585585
///
586-
/// ### Why include length?
586+
/// # Why include length?
587587
///
588588
/// A pure 96-bit content comparison can’t distinguish between two values whose inline bytes
589589
/// compare equal—either because one is a true prefix of the other or because zero-padding
@@ -605,29 +605,85 @@ impl<T: ByteViewType + ?Sized> GenericByteViewArray<T> {
605605
/// key("bar\0") = 0x0000000000000000000062617200000004
606606
/// ⇒ key("bar") < key("bar\0")
607607
/// ```
608+
/// # Inlining and Endianness
609+
///
610+
/// - We start by calling `.to_le_bytes()` on the `raw` `u128`, because Rust’s native in‑memory
611+
/// representation is little‑endian on x86/ARM.
612+
/// - We extract the low 32 bits numerically (`raw as u32`)—this step is endianness‑free.
613+
/// - We copy the 12 bytes of inline data (original order) into `buf[0..12]`.
614+
/// - We serialize `length` as big‑endian into `buf[12..16]`.
615+
/// - Finally, `u128::from_be_bytes(buf)` treats `buf[0]` as the most significant byte
616+
/// and `buf[15]` as the least significant, producing a `u128` whose integer value
617+
/// directly encodes “inline data then length” in big‑endian form.
618+
///
619+
/// This ensures that a simple `u128` comparison is equivalent to the desired
620+
/// lexicographical comparison of the inline bytes followed by length.
608621
#[inline(always)]
609622
pub fn inline_key_fast(raw: u128) -> u128 {
610-
// Convert the raw u128 (little-endian) into bytes for manipulation
623+
// 1. Decompose `raw` into little‑endian bytes:
624+
// - raw_bytes[0..4] = length in LE
625+
// - raw_bytes[4..16] = inline string data
611626
let raw_bytes = raw.to_le_bytes();
612627

613-
// Extract the length (first 4 bytes), convert to big-endian u32, and promote to u128
614-
let len_le = &raw_bytes[0..4];
615-
let len_be = u32::from_le_bytes(len_le.try_into().unwrap()).to_be() as u128;
616-
617-
// Extract the inline string bytes (next 12 bytes), place them into the lower 12 bytes of a 16-byte array,
618-
// padding the upper 4 bytes with zero to form a little-endian u128 value
619-
let mut inline_bytes = [0u8; 16];
620-
inline_bytes[4..16].copy_from_slice(&raw_bytes[4..16]);
621-
622-
// Convert to big-endian to ensure correct lexical ordering
623-
let inline_u128 = u128::from_le_bytes(inline_bytes).to_be();
624-
625-
// Shift right by 32 bits to discard the zero padding (upper 4 bytes),
626-
// so that the inline string occupies the high 96 bits
627-
let inline_part = inline_u128 >> 32;
628-
629-
// Combine the inline string part (high 96 bits) and length (low 32 bits) into the final key
630-
(inline_part << 32) | len_be
628+
// 2. Numerically truncate to get the low 32‑bit length (endianness‑free).
629+
let length = raw as u32;
630+
631+
// 3. Build a 16‑byte buffer in big‑endian order:
632+
// - buf[0..12] = inline string bytes (in original order)
633+
// - buf[12..16] = length.to_be_bytes() (BE)
634+
let mut buf = [0u8; 16];
635+
buf[0..12].copy_from_slice(&raw_bytes[4..16]); // inline data
636+
637+
// Why convert length to big-endian for comparison?
638+
//
639+
// Rust (on most platforms) stores integers in little-endian format,
640+
// meaning the least significant byte is at the lowest memory address.
641+
// For example, an u32 value like 0x22345677 is stored in memory as:
642+
//
643+
// [0x77, 0x56, 0x34, 0x22] // little-endian layout
644+
// ^ ^ ^ ^
645+
// LSB ↑↑↑ MSB
646+
//
647+
// This layout is efficient for arithmetic but *not* suitable for
648+
// lexicographic (dictionary-style) comparison of byte arrays.
649+
//
650+
// To compare values by byte order—e.g., for sorted keys or binary trees—
651+
// we must convert them to **big-endian**, where:
652+
//
653+
// - The most significant byte (MSB) comes first (index 0)
654+
// - The least significant byte (LSB) comes last (index N-1)
655+
//
656+
// In big-endian, the same u32 = 0x22345677 would be represented as:
657+
//
658+
// [0x22, 0x34, 0x56, 0x77]
659+
//
660+
// This ordering aligns with natural string/byte sorting, so calling
661+
// `.to_be_bytes()` allows us to construct
662+
// keys where standard numeric comparison (e.g., `<`, `>`) behaves
663+
// like lexicographic byte comparison.
664+
buf[12..16].copy_from_slice(&length.to_be_bytes()); // length in BE
665+
666+
// 4. Deserialize the buffer as a big‑endian u128:
667+
// buf[0] is MSB, buf[15] is LSB.
668+
// Details:
669+
// Note on endianness and layout:
670+
//
671+
// Although `buf[0]` is stored at the lowest memory address,
672+
// calling `u128::from_be_bytes(buf)` interprets it as the **most significant byte (MSB)**,
673+
// and `buf[15]` as the **least significant byte (LSB)**.
674+
//
675+
// This is the core principle of **big-endian decoding**:
676+
// - Byte at index 0 maps to bits 127..120 (highest)
677+
// - Byte at index 1 maps to bits 119..112
678+
// - ...
679+
// - Byte at index 15 maps to bits 7..0 (lowest)
680+
//
681+
// So even though memory layout goes from low to high (left to right),
682+
// big-endian treats the **first byte** as highest in value.
683+
//
684+
// This guarantees that comparing two `u128` keys is equivalent to lexicographically
685+
// comparing the original inline bytes, followed by length.
686+
u128::from_be_bytes(buf)
631687
}
632688
}
633689

@@ -1164,22 +1220,35 @@ mod tests {
11641220
///
11651221
/// This also includes a specific test for the “bar” vs. “bar\0” case, demonstrating why
11661222
/// the length field is required even when all inline bytes fit in 12 bytes.
1223+
///
1224+
/// The test includes strings that verify correct byte order (prevent reversal bugs),
1225+
/// and length-based tie-breaking in the composite key.
1226+
///
1227+
/// The test confirms that `inline_key_fast` produces keys which sort consistently
1228+
/// with the expected lexicographical order of the raw byte arrays.
11671229
#[test]
11681230
fn test_inline_key_fast_various_lengths_and_lexical() {
1169-
/// Helper to create a raw u128 value representing an inline ByteView
1170-
/// - `length`: number of meaningful bytes (≤ 12)
1171-
/// - `data`: the actual inline data
1231+
/// Helper to create a raw u128 value representing an inline ByteView:
1232+
/// - `length`: number of meaningful bytes (must be ≤ 12)
1233+
/// - `data`: the actual inline data bytes
1234+
///
1235+
/// The first 4 bytes encode length in little-endian,
1236+
/// the following 12 bytes contain the inline string data (unpadded).
11721237
fn make_raw_inline(length: u32, data: &[u8]) -> u128 {
11731238
assert!(length as usize <= 12, "Inline length must be ≤ 12");
1174-
assert!(data.len() == length as usize, "Data must match length");
1239+
assert!(
1240+
data.len() == length as usize,
1241+
"Data length must match `length`"
1242+
);
11751243

11761244
let mut raw_bytes = [0u8; 16];
1177-
raw_bytes[0..4].copy_from_slice(&length.to_le_bytes()); // little-endian length
1245+
raw_bytes[0..4].copy_from_slice(&length.to_le_bytes()); // length stored little-endian
11781246
raw_bytes[4..(4 + data.len())].copy_from_slice(data); // inline data
11791247
u128::from_le_bytes(raw_bytes)
11801248
}
11811249

1182-
// Test inputs: include the specific "bar" vs "bar\0" case, plus length and lexical variations
1250+
// Test inputs: various lengths and lexical orders,
1251+
// plus special cases for byte order and length tie-breaking
11831252
let test_inputs: Vec<&[u8]> = vec![
11841253
b"a",
11851254
b"aa",
@@ -1193,23 +1262,42 @@ mod tests {
11931262
b"abcdefghi",
11941263
b"abcdefghij",
11951264
b"abcdefghijk",
1196-
b"abcdefghijkl", // 12 bytes, max inline
1265+
b"abcdefghijkl",
1266+
// Tests for byte-order reversal bug:
1267+
// Without the fix, "backend one" would compare as "eno dnekcab",
1268+
// causing incorrect sort order relative to "backend two".
1269+
b"backend one",
1270+
b"backend two",
1271+
// Tests length-tiebreaker logic:
1272+
// "bar" (3 bytes) and "bar\0" (4 bytes) have identical inline data,
1273+
// so only the length differentiates their ordering.
11971274
b"bar",
1198-
b"bar\0", // special case to test length tiebreaker
1275+
b"bar\0",
1276+
// Additional lexical and length tie-breaking cases with same prefix, in correct lex order:
1277+
b"than12Byt",
1278+
b"than12Bytes",
1279+
b"than12Bytes\0",
1280+
b"than12Bytesx",
1281+
b"than12Bytex",
1282+
b"than12Bytez",
1283+
// Additional lexical tests
11991284
b"xyy",
12001285
b"xyz",
1286+
b"xza",
12011287
];
12021288

1203-
// Monotonic key order: content then length,and cross-check against GenericBinaryArray comparison
1289+
// Create a GenericBinaryArray for cross-comparison of lex order
12041290
let array: GenericBinaryArray<i32> =
12051291
GenericBinaryArray::from(test_inputs.iter().map(|s| Some(*s)).collect::<Vec<_>>());
12061292

12071293
for i in 0..array.len() - 1 {
12081294
let v1 = array.value(i);
12091295
let v2 = array.value(i + 1);
1210-
// Ensure lexical ordering matches
1296+
1297+
// Assert the array's natural lexical ordering is correct
12111298
assert!(v1 < v2, "Array compare failed: {v1:?} !< {v2:?}");
1212-
// Ensure fast key compare matches
1299+
1300+
// Assert the keys produced by inline_key_fast reflect the same ordering
12131301
let key1 = GenericByteViewArray::<BinaryViewType>::inline_key_fast(make_raw_inline(
12141302
v1.len() as u32,
12151303
v1,
@@ -1218,6 +1306,7 @@ mod tests {
12181306
v2.len() as u32,
12191307
v2,
12201308
));
1309+
12211310
assert!(
12221311
key1 < key2,
12231312
"Key compare failed: key({v1:?})=0x{key1:032x} !< key({v2:?})=0x{key2:032x}",

0 commit comments

Comments
 (0)