subject:"Re\: \[PR\] \[WIp\] Alp encoding support \[arrow\-rs\]"

Re: [PR] [WIp] Alp encoding support [arrow-rs]

2026-05-08 Thread via GitHub



alamb commented on code in PR #9372:
URL: https://github.com/apache/arrow-rs/pull/9372#discussion_r3208770338


##
parquet/src/encodings/decoding/alp.rs:
##
@@ -0,0 +1,1258 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::marker::PhantomData;
+use std::ops::Range;
+
+use bytes::Bytes;
+
+use crate::basic::Encoding;
+use crate::data_type::DataType;
+use crate::encodings::decoding::Decoder;
+use crate::errors::{ParquetError, Result};
+use crate::util::bit_util::{BitReader, FromBytes};
+
+const ALP_HEADER_SIZE: usize = 8;
+const ALP_VERSION: u8 = 1;
+const ALP_COMPRESSION_MODE: u8 = 0;
+const ALP_INTEGER_ENCODING_FOR_BIT_PACK: u8 = 0;
+const ALP_MAX_LOG_VECTOR_SIZE: u8 = 16;
+const ALP_MAX_EXPONENT_F32: u8 = 10;
+const ALP_MAX_EXPONENT_F64: u8 = 18;
+
+/// Page-level ALP header (version 1, 8 bytes).
+///
+/// Layout in bytes:
+/// - `[0]` `version`
+/// - `[1]` `compression_mode`
+/// - `[2]` `integer_encoding`
+/// - `[3]` `log_vector_size`
+/// - `[4..8]` `num_elements` (little-endian `i32`)
+#[derive(Debug, Clone, Copy)]
+struct AlpHeader {
+version: u8,
+compression_mode: u8,
+integer_encoding: u8,
+log_vector_size: u8,
+num_elements: i32,
+}
+
+impl AlpHeader {
+fn num_elements_usize(&self) -> usize {
+self.num_elements as usize
+}
+
+fn vector_size(&self) -> usize {
+1usize << self.log_vector_size
+}
+
+fn num_vectors(&self) -> usize {
+if self.num_elements == 0 {
+0
+} else {
+self.num_elements_usize().div_ceil(self.vector_size())
+}
+}
+
+fn vector_num_elements(&self, vector_index: usize) -> u16 {
+let vector_size = self.vector_size();
+let num_full_vectors = self.num_elements_usize() / vector_size;
+let remainder = self.num_elements_usize() % vector_size;
+if vector_index < num_full_vectors {
+vector_size as u16

Review Comment:
   I personally recommend changing all computation of offsets to use `usize` -- 
e.g. here return a usize
   
   Then we can carefully check overflow, etc on write / read once 
   



##
parquet/src/encodings/decoding/alp.rs:
##
@@ -0,0 +1,1524 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::marker::PhantomData;
+use std::ops::Range;
+
+use bytes::Bytes;
+
+use crate::basic::Encoding;
+use crate::data_type::DataType;
+use crate::encodings::decoding::Decoder;
+use crate::errors::{ParquetError, Result};
+use crate::util::bit_util::{BitReader, FromBytes};
+
+const ALP_HEADER_SIZE: usize = 7;
+const ALP_COMPRESSION_MODE: u8 = 0;
+const ALP_INTEGER_ENCODING_FOR_BIT_PACK: u8 = 0;
+const ALP_MIN_LOG_VECTOR_SIZE: u8 = 3;
+const ALP_MAX_LOG_VECTOR_SIZE: u8 = 15;
+const ALP_MAX_EXPONENT_F32: u8 = 10;
+const ALP_MAX_EXPONENT_F64: u8 = 18;
+
+/// Page-level ALP header (7 bytes).
+///
+/// Layout in bytes:
+/// - `[0]` `compression_mode`
+/// - `[1]` `integer_encoding`
+/// - `[2]` `log_vector_size`
+/// - `[3..7]` `num_elements` (little-endian `i32`)
+#[derive(Debug, Clone, Copy)]
+struct AlpHeader {
+compression_mode: u8,
+integer_encoding: u8,
+log_vector_size: u8,
+num_elements: i32,
+}
+
+impl AlpHeader {
+fn num_elements_usize(&self) -> usize {
+self.num_elements as usize
+}
+
+fn vector_size(&self) -> usize {
+1usize << self.log_vector_size
+}
+
+fn num_vectors(&self) -> usize {
+if self.num_elements

Re: [PR] [WIp] Alp encoding support [arrow-rs]

2026-05-08 Thread via GitHub



alamb commented on PR #9372:
URL: https://github.com/apache/arrow-rs/pull/9372#issuecomment-4405914345

   Merged up to 58.2.0 


-- 
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]

Re: [PR] [WIp] Alp encoding support [arrow-rs]

2026-05-08 Thread via GitHub



alamb commented on PR #9372:
URL: https://github.com/apache/arrow-rs/pull/9372#issuecomment-4405521374

   I am starting to check this one out


-- 
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]

Re: [PR] [WIp] Alp encoding support [arrow-rs]

2026-05-06 Thread via GitHub



alamb commented on PR #9372:
URL: https://github.com/apache/arrow-rs/pull/9372#issuecomment-4390376355

   I reviewed the format PR last week
   - https://github.com/apache/parquet-format/pull/557
   
   I am now hoping to find time to run through this PR for details


-- 
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]

Re: [PR] [WIp] Alp encoding support [arrow-rs]

2026-04-20 Thread via GitHub



alamb commented on PR #9372:
URL: https://github.com/apache/arrow-rs/pull/9372#issuecomment-4281258639

   Tanks @devanbenz and @sdf-jkl 
   
   I will try and check this one out in a bit.
   
   


-- 
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]

Re: [PR] [WIp] Alp encoding support [arrow-rs]

2026-04-16 Thread via GitHub



devanbenz commented on PR #9372:
URL: https://github.com/apache/arrow-rs/pull/9372#issuecomment-4264516905

   > @devanbenz Could you take another look? Thanks!
   
   All my comments are addressed. Thank you! 
   
   We'll need @alamb to pass through for a final review 👍 


-- 
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]

Re: [PR] [WIp] Alp encoding support [arrow-rs]

2026-04-02 Thread via GitHub



sdf-jkl commented on PR #9372:
URL: https://github.com/apache/arrow-rs/pull/9372#issuecomment-4180105802

   @devanbenz Could you take another look? Thanks!


-- 
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]

Re: [PR] [WIp] Alp encoding support [arrow-rs]

2026-03-12 Thread via GitHub



sdf-jkl commented on PR #9372:
URL: https://github.com/apache/arrow-rs/pull/9372#issuecomment-4050905875

   I swear I did :crying_cat_face: 


-- 
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]

Re: [PR] [WIp] Alp encoding support [arrow-rs]

2026-03-12 Thread via GitHub



devanbenz commented on PR #9372:
URL: https://github.com/apache/arrow-rs/pull/9372#issuecomment-4050789059

   > Thanks @devanbenz, I finally addressed your reviews! 
   
   Please cargo fmt the code 🫡


-- 
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]

Re: [PR] [WIp] Alp encoding support [arrow-rs]

2026-03-12 Thread via GitHub



sdf-jkl commented on PR #9372:
URL: https://github.com/apache/arrow-rs/pull/9372#issuecomment-4049870986

   Thanks @devanbenz, I finally addressed your reviews! 


-- 
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]

Re: [PR] [WIp] Alp encoding support [arrow-rs]

2026-03-12 Thread via GitHub



sdf-jkl commented on code in PR #9372:
URL: https://github.com/apache/arrow-rs/pull/9372#discussion_r2927230325


##
parquet/src/encodings/decoding/alp.rs:
##
@@ -0,0 +1,1258 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::marker::PhantomData;
+use std::ops::Range;
+
+use bytes::Bytes;
+
+use crate::basic::Encoding;
+use crate::data_type::DataType;
+use crate::encodings::decoding::Decoder;
+use crate::errors::{ParquetError, Result};
+use crate::util::bit_util::{BitReader, FromBytes};
+
+const ALP_HEADER_SIZE: usize = 8;
+const ALP_VERSION: u8 = 1;
+const ALP_COMPRESSION_MODE: u8 = 0;
+const ALP_INTEGER_ENCODING_FOR_BIT_PACK: u8 = 0;
+const ALP_MAX_LOG_VECTOR_SIZE: u8 = 16;
+const ALP_MAX_EXPONENT_F32: u8 = 10;
+const ALP_MAX_EXPONENT_F64: u8 = 18;
+
+/// Page-level ALP header (version 1, 8 bytes).
+///
+/// Layout in bytes:
+/// - `[0]` `version`
+/// - `[1]` `compression_mode`
+/// - `[2]` `integer_encoding`
+/// - `[3]` `log_vector_size`
+/// - `[4..8]` `num_elements` (little-endian `i32`)
+#[derive(Debug, Clone, Copy)]
+struct AlpHeader {
+version: u8,
+compression_mode: u8,
+integer_encoding: u8,
+log_vector_size: u8,
+num_elements: i32,
+}
+
+impl AlpHeader {
+fn num_elements_usize(&self) -> usize {
+self.num_elements as usize
+}
+
+fn vector_size(&self) -> usize {
+1usize << self.log_vector_size
+}
+
+fn num_vectors(&self) -> usize {
+if self.num_elements == 0 {
+0
+} else {
+self.num_elements_usize().div_ceil(self.vector_size())
+}
+}
+
+fn vector_num_elements(&self, vector_index: usize) -> u16 {
+let vector_size = self.vector_size();
+let num_full_vectors = self.num_elements_usize() / vector_size;
+let remainder = self.num_elements_usize() % vector_size;
+if vector_index < num_full_vectors {
+vector_size as u16
+} else if vector_index == num_full_vectors && remainder > 0 {
+remainder as u16
+} else {
+0
+}
+}
+}
+
+/// Per-vector ALP metadata (4 bytes), equivalent to C++ 
`AlpEncodedVectorInfo`.
+#[derive(Debug, Clone, Copy)]
+struct AlpEncodedVectorInfo {
+exponent: u8,
+factor: u8,
+num_exceptions: u16,
+}
+
+impl AlpEncodedVectorInfo {
+const STORED_SIZE: usize = 4;
+}
+
+/// Per-vector FOR metadata for exact integer type (`u32` for `f32`, `u64` for 
`f64`).
+#[derive(Debug, Clone, Copy)]
+struct AlpEncodedForVectorInfo {
+frame_of_reference: Exact,
+bit_width: u8,
+}
+
+impl AlpEncodedForVectorInfo {
+fn stored_size() -> usize {
+Exact::WIDTH + 1
+}
+
+fn get_bit_packed_size(&self, num_elements: u16) -> usize {
+(self.bit_width as usize * num_elements as usize).div_ceil(8)
+}
+
+fn get_data_stored_size(&self, num_elements: u16, num_exceptions: u16) -> 
usize {
+let bit_packed_size = self.get_bit_packed_size(num_elements);
+bit_packed_size
++ num_exceptions as usize * std::mem::size_of::()
++ num_exceptions as usize * Exact::WIDTH
+}
+}
+
+/// Parsed view of one vector's metadata and data slices.
+///
+/// `packed_values` is a zero-copy range into page body bytes.
+/// Exception positions/values are copied for straightforward decode handling.
+#[derive(Debug)]
+struct AlpEncodedVectorView {
+num_elements: u16,
+alp_info: AlpEncodedVectorInfo,
+for_info: AlpEncodedForVectorInfo,
+packed_values: Range,
+exception_positions: Vec,
+exception_values: Vec,
+}
+
+/// Parsed ALP page layout for one exact integer width (`u32` for float pages,
+/// `u64` for double pages).
+#[derive(Debug)]
+struct AlpPageLayout {
+header: AlpHeader,
+body: Bytes,
+offsets: Vec,
+}
+
+/// Exact integer type used by FOR reconstruction.
+///
+/// This mirrors C++:
+/// - `float`  -> `uint32_t`
+/// - `double` -> `uint64_t`
+///
+/// Why unsigned (not `i32`/`i64`)?
+/// - FOR stores non-negative deltas optimized for bitpacking.
+/// - Unsigned arithmetic avoids signed-overflow edge cases in FOR stage.
+/// - Signed interpretation is applied later during decimal reconstruction.
+pub(super) trait Al

Re: [PR] [WIp] Alp encoding support [arrow-rs]

2026-03-12 Thread via GitHub



sdf-jkl commented on code in PR #9372:
URL: https://github.com/apache/arrow-rs/pull/9372#discussion_r2927229145


##
parquet/src/encodings/decoding/alp.rs:
##
@@ -0,0 +1,1258 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::marker::PhantomData;
+use std::ops::Range;
+
+use bytes::Bytes;
+
+use crate::basic::Encoding;
+use crate::data_type::DataType;
+use crate::encodings::decoding::Decoder;
+use crate::errors::{ParquetError, Result};
+use crate::util::bit_util::{BitReader, FromBytes};
+
+const ALP_HEADER_SIZE: usize = 8;
+const ALP_VERSION: u8 = 1;
+const ALP_COMPRESSION_MODE: u8 = 0;
+const ALP_INTEGER_ENCODING_FOR_BIT_PACK: u8 = 0;
+const ALP_MAX_LOG_VECTOR_SIZE: u8 = 16;
+const ALP_MAX_EXPONENT_F32: u8 = 10;
+const ALP_MAX_EXPONENT_F64: u8 = 18;
+
+/// Page-level ALP header (version 1, 8 bytes).
+///
+/// Layout in bytes:
+/// - `[0]` `version`
+/// - `[1]` `compression_mode`
+/// - `[2]` `integer_encoding`
+/// - `[3]` `log_vector_size`
+/// - `[4..8]` `num_elements` (little-endian `i32`)
+#[derive(Debug, Clone, Copy)]
+struct AlpHeader {
+version: u8,
+compression_mode: u8,
+integer_encoding: u8,
+log_vector_size: u8,
+num_elements: i32,
+}
+
+impl AlpHeader {
+fn num_elements_usize(&self) -> usize {
+self.num_elements as usize
+}
+
+fn vector_size(&self) -> usize {
+1usize << self.log_vector_size
+}
+
+fn num_vectors(&self) -> usize {
+if self.num_elements == 0 {
+0
+} else {
+self.num_elements_usize().div_ceil(self.vector_size())
+}
+}
+
+fn vector_num_elements(&self, vector_index: usize) -> u16 {
+let vector_size = self.vector_size();
+let num_full_vectors = self.num_elements_usize() / vector_size;
+let remainder = self.num_elements_usize() % vector_size;
+if vector_index < num_full_vectors {
+vector_size as u16
+} else if vector_index == num_full_vectors && remainder > 0 {
+remainder as u16
+} else {
+0
+}
+}
+}
+
+/// Per-vector ALP metadata (4 bytes), equivalent to C++ 
`AlpEncodedVectorInfo`.
+#[derive(Debug, Clone, Copy)]
+struct AlpEncodedVectorInfo {
+exponent: u8,
+factor: u8,
+num_exceptions: u16,
+}
+
+impl AlpEncodedVectorInfo {
+const STORED_SIZE: usize = 4;
+}
+
+/// Per-vector FOR metadata for exact integer type (`u32` for `f32`, `u64` for 
`f64`).
+#[derive(Debug, Clone, Copy)]
+struct AlpEncodedForVectorInfo {
+frame_of_reference: Exact,
+bit_width: u8,
+}
+
+impl AlpEncodedForVectorInfo {
+fn stored_size() -> usize {
+Exact::WIDTH + 1
+}
+
+fn get_bit_packed_size(&self, num_elements: u16) -> usize {
+(self.bit_width as usize * num_elements as usize).div_ceil(8)
+}
+
+fn get_data_stored_size(&self, num_elements: u16, num_exceptions: u16) -> 
usize {
+let bit_packed_size = self.get_bit_packed_size(num_elements);
+bit_packed_size
++ num_exceptions as usize * std::mem::size_of::()
++ num_exceptions as usize * Exact::WIDTH
+}
+}
+
+/// Parsed view of one vector's metadata and data slices.
+///
+/// `packed_values` is a zero-copy range into page body bytes.
+/// Exception positions/values are copied for straightforward decode handling.
+#[derive(Debug)]
+struct AlpEncodedVectorView {
+num_elements: u16,
+alp_info: AlpEncodedVectorInfo,
+for_info: AlpEncodedForVectorInfo,
+packed_values: Range,
+exception_positions: Vec,
+exception_values: Vec,
+}
+
+/// Parsed ALP page layout for one exact integer width (`u32` for float pages,
+/// `u64` for double pages).
+#[derive(Debug)]
+struct AlpPageLayout {
+header: AlpHeader,
+body: Bytes,
+offsets: Vec,
+}
+
+/// Exact integer type used by FOR reconstruction.
+///
+/// This mirrors C++:
+/// - `float`  -> `uint32_t`
+/// - `double` -> `uint64_t`
+///
+/// Why unsigned (not `i32`/`i64`)?
+/// - FOR stores non-negative deltas optimized for bitpacking.
+/// - Unsigned arithmetic avoids signed-overflow edge cases in FOR stage.
+/// - Signed interpretation is applied later during decimal reconstruction.
+pub(super) trait Al

Re: [PR] [WIp] Alp encoding support [arrow-rs]

2026-03-12 Thread via GitHub



sdf-jkl commented on code in PR #9372:
URL: https://github.com/apache/arrow-rs/pull/9372#discussion_r2927227780


##
parquet/src/encodings/decoding/alp.rs:
##
@@ -0,0 +1,1258 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::marker::PhantomData;
+use std::ops::Range;
+
+use bytes::Bytes;
+
+use crate::basic::Encoding;
+use crate::data_type::DataType;
+use crate::encodings::decoding::Decoder;
+use crate::errors::{ParquetError, Result};
+use crate::util::bit_util::{BitReader, FromBytes};
+
+const ALP_HEADER_SIZE: usize = 8;
+const ALP_VERSION: u8 = 1;
+const ALP_COMPRESSION_MODE: u8 = 0;
+const ALP_INTEGER_ENCODING_FOR_BIT_PACK: u8 = 0;
+const ALP_MAX_LOG_VECTOR_SIZE: u8 = 16;
+const ALP_MAX_EXPONENT_F32: u8 = 10;
+const ALP_MAX_EXPONENT_F64: u8 = 18;
+
+/// Page-level ALP header (version 1, 8 bytes).
+///
+/// Layout in bytes:
+/// - `[0]` `version`
+/// - `[1]` `compression_mode`
+/// - `[2]` `integer_encoding`
+/// - `[3]` `log_vector_size`
+/// - `[4..8]` `num_elements` (little-endian `i32`)
+#[derive(Debug, Clone, Copy)]
+struct AlpHeader {
+version: u8,
+compression_mode: u8,
+integer_encoding: u8,
+log_vector_size: u8,
+num_elements: i32,
+}
+
+impl AlpHeader {
+fn num_elements_usize(&self) -> usize {
+self.num_elements as usize
+}
+
+fn vector_size(&self) -> usize {
+1usize << self.log_vector_size
+}
+
+fn num_vectors(&self) -> usize {
+if self.num_elements == 0 {
+0
+} else {
+self.num_elements_usize().div_ceil(self.vector_size())
+}
+}
+
+fn vector_num_elements(&self, vector_index: usize) -> u16 {
+let vector_size = self.vector_size();
+let num_full_vectors = self.num_elements_usize() / vector_size;
+let remainder = self.num_elements_usize() % vector_size;
+if vector_index < num_full_vectors {
+vector_size as u16

Review Comment:
   fixed 
https://github.com/apache/arrow-rs/pull/9372/commits/a13f24425646025d2e6f86714d106b00108f6111



-- 
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]

Re: [PR] [WIp] Alp encoding support [arrow-rs]

2026-03-12 Thread via GitHub



sdf-jkl commented on code in PR #9372:
URL: https://github.com/apache/arrow-rs/pull/9372#discussion_r2927225890


##
parquet/src/encodings/decoding/alp.rs:
##
@@ -0,0 +1,1258 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::marker::PhantomData;
+use std::ops::Range;
+
+use bytes::Bytes;
+
+use crate::basic::Encoding;
+use crate::data_type::DataType;
+use crate::encodings::decoding::Decoder;
+use crate::errors::{ParquetError, Result};
+use crate::util::bit_util::{BitReader, FromBytes};
+
+const ALP_HEADER_SIZE: usize = 8;
+const ALP_VERSION: u8 = 1;
+const ALP_COMPRESSION_MODE: u8 = 0;
+const ALP_INTEGER_ENCODING_FOR_BIT_PACK: u8 = 0;
+const ALP_MAX_LOG_VECTOR_SIZE: u8 = 16;
+const ALP_MAX_EXPONENT_F32: u8 = 10;
+const ALP_MAX_EXPONENT_F64: u8 = 18;
+
+/// Page-level ALP header (version 1, 8 bytes).

Review Comment:
   fixed 
https://github.com/apache/arrow-rs/pull/9372/commits/6b7e923ede8b3e9742a5488a2a26aea9c5c32535



-- 
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]

Re: [PR] [WIp] Alp encoding support [arrow-rs]

2026-03-12 Thread via GitHub



sdf-jkl commented on code in PR #9372:
URL: https://github.com/apache/arrow-rs/pull/9372#discussion_r2926502631


##
parquet/src/encodings/decoding/alp.rs:
##
@@ -0,0 +1,1258 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::marker::PhantomData;
+use std::ops::Range;
+
+use bytes::Bytes;
+
+use crate::basic::Encoding;
+use crate::data_type::DataType;
+use crate::encodings::decoding::Decoder;
+use crate::errors::{ParquetError, Result};
+use crate::util::bit_util::{BitReader, FromBytes};
+
+const ALP_HEADER_SIZE: usize = 8;
+const ALP_VERSION: u8 = 1;
+const ALP_COMPRESSION_MODE: u8 = 0;
+const ALP_INTEGER_ENCODING_FOR_BIT_PACK: u8 = 0;
+const ALP_MAX_LOG_VECTOR_SIZE: u8 = 16;
+const ALP_MAX_EXPONENT_F32: u8 = 10;
+const ALP_MAX_EXPONENT_F64: u8 = 18;
+
+/// Page-level ALP header (version 1, 8 bytes).
+///
+/// Layout in bytes:
+/// - `[0]` `version`
+/// - `[1]` `compression_mode`
+/// - `[2]` `integer_encoding`
+/// - `[3]` `log_vector_size`
+/// - `[4..8]` `num_elements` (little-endian `i32`)
+#[derive(Debug, Clone, Copy)]
+struct AlpHeader {
+version: u8,
+compression_mode: u8,
+integer_encoding: u8,
+log_vector_size: u8,
+num_elements: i32,
+}
+
+impl AlpHeader {
+fn num_elements_usize(&self) -> usize {
+self.num_elements as usize
+}
+
+fn vector_size(&self) -> usize {
+1usize << self.log_vector_size
+}
+
+fn num_vectors(&self) -> usize {
+if self.num_elements == 0 {
+0
+} else {
+self.num_elements_usize().div_ceil(self.vector_size())
+}
+}
+
+fn vector_num_elements(&self, vector_index: usize) -> u16 {
+let vector_size = self.vector_size();
+let num_full_vectors = self.num_elements_usize() / vector_size;
+let remainder = self.num_elements_usize() % vector_size;
+if vector_index < num_full_vectors {
+vector_size as u16

Review Comment:
   covered in the new spec: 
https://github.com/apache/parquet-format/pull/557/changes#diff-8483dcc0e62802dbfd5128dec4c128f78acd1f755e4143b7234d651276f436efR454



-- 
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]

Re: [PR] [WIp] Alp encoding support [arrow-rs]

2026-03-11 Thread via GitHub



devanbenz commented on code in PR #9372:
URL: https://github.com/apache/arrow-rs/pull/9372#discussion_r2921214327


##
parquet/src/encodings/decoding/alp.rs:
##
@@ -0,0 +1,1258 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::marker::PhantomData;
+use std::ops::Range;
+
+use bytes::Bytes;
+
+use crate::basic::Encoding;
+use crate::data_type::DataType;
+use crate::encodings::decoding::Decoder;
+use crate::errors::{ParquetError, Result};
+use crate::util::bit_util::{BitReader, FromBytes};
+
+const ALP_HEADER_SIZE: usize = 8;
+const ALP_VERSION: u8 = 1;
+const ALP_COMPRESSION_MODE: u8 = 0;
+const ALP_INTEGER_ENCODING_FOR_BIT_PACK: u8 = 0;
+const ALP_MAX_LOG_VECTOR_SIZE: u8 = 16;
+const ALP_MAX_EXPONENT_F32: u8 = 10;
+const ALP_MAX_EXPONENT_F64: u8 = 18;
+
+/// Page-level ALP header (version 1, 8 bytes).

Review Comment:
   Header here is 8 bytes, it is 7 bytes in the official spec: 
https://github.com/apache/parquet-format/pull/557/changes#diff-8483dcc0e62802dbfd5128dec4c128f78acd1f755e4143b7234d651276f436efR391
   
   No version required in the header per the spec. You need 
   - compression_mode
   - integer_encoding
   - log_vector_size
   - num_elements



-- 
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]

Re: [PR] [WIp] Alp encoding support [arrow-rs]

2026-03-11 Thread via GitHub



alamb commented on PR #9372:
URL: https://github.com/apache/arrow-rs/pull/9372#issuecomment-4041094995

   The final spec is 
   - https://github.com/apache/parquet-format/pull/557


-- 
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]

Re: [PR] [WIp] Alp encoding support [arrow-rs]

2026-03-10 Thread via GitHub



sdf-jkl commented on PR #9372:
URL: https://github.com/apache/arrow-rs/pull/9372#issuecomment-4034343340

   @devanbenz Sorry for taking so long to address your comments. I'll make sure 
to work on it this week!


-- 
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]

Re: [PR] [WIp] Alp encoding support [arrow-rs]

2026-03-02 Thread via GitHub



sdf-jkl commented on code in PR #9372:
URL: https://github.com/apache/arrow-rs/pull/9372#discussion_r2875012335


##
parquet/src/encodings/decoding/alp.rs:
##
@@ -0,0 +1,1258 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::marker::PhantomData;
+use std::ops::Range;
+
+use bytes::Bytes;
+
+use crate::basic::Encoding;
+use crate::data_type::DataType;
+use crate::encodings::decoding::Decoder;
+use crate::errors::{ParquetError, Result};
+use crate::util::bit_util::{BitReader, FromBytes};
+
+const ALP_HEADER_SIZE: usize = 8;
+const ALP_VERSION: u8 = 1;
+const ALP_COMPRESSION_MODE: u8 = 0;
+const ALP_INTEGER_ENCODING_FOR_BIT_PACK: u8 = 0;
+const ALP_MAX_LOG_VECTOR_SIZE: u8 = 16;
+const ALP_MAX_EXPONENT_F32: u8 = 10;
+const ALP_MAX_EXPONENT_F64: u8 = 18;
+
+/// Page-level ALP header (version 1, 8 bytes).
+///
+/// Layout in bytes:
+/// - `[0]` `version`
+/// - `[1]` `compression_mode`
+/// - `[2]` `integer_encoding`
+/// - `[3]` `log_vector_size`
+/// - `[4..8]` `num_elements` (little-endian `i32`)
+#[derive(Debug, Clone, Copy)]
+struct AlpHeader {
+version: u8,
+compression_mode: u8,
+integer_encoding: u8,
+log_vector_size: u8,
+num_elements: i32,
+}
+
+impl AlpHeader {
+fn num_elements_usize(&self) -> usize {
+self.num_elements as usize
+}
+
+fn vector_size(&self) -> usize {
+1usize << self.log_vector_size
+}
+
+fn num_vectors(&self) -> usize {
+if self.num_elements == 0 {
+0
+} else {
+self.num_elements_usize().div_ceil(self.vector_size())
+}
+}
+
+fn vector_num_elements(&self, vector_index: usize) -> u16 {
+let vector_size = self.vector_size();
+let num_full_vectors = self.num_elements_usize() / vector_size;
+let remainder = self.num_elements_usize() % vector_size;
+if vector_index < num_full_vectors {
+vector_size as u16

Review Comment:
   thanks, I asked a question about this in the original C++ PR - 
https://github.com/apache/arrow/pull/48345#discussion_r2835501958



-- 
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]

Re: [PR] [WIp] Alp encoding support [arrow-rs]

2026-02-20 Thread via GitHub



sdf-jkl commented on PR #9372:
URL: https://github.com/apache/arrow-rs/pull/9372#issuecomment-3937594022

   @alamb I worked with codex on replicating the c++ implementation reviewing 
commit by commit. 
   
   I'll do one final read myself, but this should be ready for initial review.


-- 
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]

Re: [PR] [WIp] Alp encoding support [arrow-rs]

2026-02-08 Thread via GitHub



alamb commented on PR #9372:
URL: https://github.com/apache/arrow-rs/pull/9372#issuecomment-3867178621

   amaaazing


-- 
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]

Re: [PR] [WIp] Alp encoding support [arrow-rs]

Re: [PR] [WIp] Alp encoding support [arrow-rs]

Re: [PR] [WIp] Alp encoding support [arrow-rs]

Re: [PR] [WIp] Alp encoding support [arrow-rs]

Re: [PR] [WIp] Alp encoding support [arrow-rs]

Re: [PR] [WIp] Alp encoding support [arrow-rs]

Re: [PR] [WIp] Alp encoding support [arrow-rs]

Re: [PR] [WIp] Alp encoding support [arrow-rs]

Re: [PR] [WIp] Alp encoding support [arrow-rs]

Re: [PR] [WIp] Alp encoding support [arrow-rs]

Re: [PR] [WIp] Alp encoding support [arrow-rs]

Re: [PR] [WIp] Alp encoding support [arrow-rs]

Re: [PR] [WIp] Alp encoding support [arrow-rs]

Re: [PR] [WIp] Alp encoding support [arrow-rs]

Re: [PR] [WIp] Alp encoding support [arrow-rs]

Re: [PR] [WIp] Alp encoding support [arrow-rs]

Re: [PR] [WIp] Alp encoding support [arrow-rs]

Re: [PR] [WIp] Alp encoding support [arrow-rs]

Re: [PR] [WIp] Alp encoding support [arrow-rs]

Re: [PR] [WIp] Alp encoding support [arrow-rs]

Re: [PR] [WIp] Alp encoding support [arrow-rs]

21 matches

Site Navigation

Mail list logo

Footer information