From c55b5a2b74e5dbc1e09a9b2b46c7ee12d73ce80a Mon Sep 17 00:00:00 2001 From: Mikhail Kot Date: Wed, 22 Jul 2026 16:22:02 +0100 Subject: [PATCH 1/2] fix --- lang/cpp/include/vortex/array.hpp | 62 ++++++- lang/cpp/src/array.cpp | 40 +++-- lang/cpp/tests/array.cpp | 40 +++++ lang/cpp/tests/expression.cpp | 13 +- vortex-ffi/cbindgen.toml | 6 + vortex-ffi/cinclude/vortex.h | 80 ++++++++- vortex-ffi/src/array.rs | 274 ++++++++++++++++++++++++++---- vortex-ffi/test/array.cpp | 105 ++++++++++++ 8 files changed, 558 insertions(+), 62 deletions(-) diff --git a/lang/cpp/include/vortex/array.hpp b/lang/cpp/include/vortex/array.hpp index 43b986f4b42..28edcc3e323 100644 --- a/lang/cpp/include/vortex/array.hpp +++ b/lang/cpp/include/vortex/array.hpp @@ -11,6 +11,7 @@ #include #include +#include #include #include #include @@ -30,6 +31,43 @@ class Array; class StringView; class BytesView; +/** + * Readonly view over bitpacked booleans. + * + * Bits are laid out LSB-first. + * "bit_offset" is in [0; 8) and lets a view start at a non-byte-aligned bit. + * + * Example: + * "view" holds 6 boolean elements starting at bit 2, first 5 set to "true", + * last is "false". + * + * uint8_t word = 0b01111100; + * BoolView view = {&word, 6, 2}; + */ +struct BoolView { + std::span bytes; + /* + * Bit offset of first element in "bytes". + * Example: if bit_offset is 2, first bit is at bytes[0] & (1 << 2). + */ + size_t bit_offset = 0; + + // Get bit at "index". Index is in [0; elements()). + inline bool operator[](size_t index) const { + return (bytes.data()[(index + bit_offset) / 8] & (1 << ((index + bit_offset) % 8))); + } + + // Number of uint8_t words storing elements. + inline size_t words() const { + return bytes.size(); + } + + // Number of set elements/bits + inline size_t elements() const { + return words() * 8 - bit_offset; + } +}; + /* * Validity type tells us whether there are null/invalid values in an Array. */ @@ -101,8 +139,7 @@ class ValidityBits { ValidityBits(const Session &session, const vx_array *canonical); const vx_array *owner_ = nullptr; - const uint8_t *bits_ = nullptr; - size_t bit_offset_ = 0; + BoolView view_; bool all_invalid_ = false; }; } // namespace detail @@ -131,6 +168,17 @@ class Array { return primitive_raw(detail::to_ptype(), data.data(), data.size(), validity); } + /** + * A Bool array copied from bitpacked storage. + * + * Example: + * + * std::vector bits(2, 1); + * BoolView view = {bits, 0}; + * auto array = Array::bool_array(view); + */ + static Array bool_array(const BoolView &view, const Validity &validity = ValidityType::NonNullable); + /** * Import an Arrow array. Consumes both "array" and "schema", do not use * or release them afterwards. For a record batch pass nullable = false. @@ -293,17 +341,17 @@ class PrimitiveView { }; /** - * Read-only view over a Bool array. As Bool values are bit-packed, there's no - * span. Read individual values with value(i). + * Read-only view over a Bool array. */ template <> class PrimitiveView { public: /* - * Get raw value from this view. Values at null/invalid positions are - * unspecified. + * Get bitpacked storage for this view's values. Values at null/invalid + * positions are unspecified */ - bool value(size_t index) const; + BoolView values() const; + bool is_null(size_t index) const { return validity_.is_null(index); } diff --git a/lang/cpp/src/array.cpp b/lang/cpp/src/array.cpp index b4adf69fe5c..7fee8e4bc7b 100644 --- a/lang/cpp/src/array.cpp +++ b/lang/cpp/src/array.cpp @@ -76,11 +76,10 @@ bool ValidityBits::is_null(size_t index) const { if (all_invalid_) { return true; } - if (bits_ == nullptr) { + if (owner_ == nullptr) { return false; } - const size_t bit = bit_offset_ + index; - return (bits_[bit / 8] >> (bit % 8) & 1) == 0; + return view_[index]; } ValidityBits::ValidityBits(const Session &session, const vx_array *canonical) { @@ -104,29 +103,27 @@ ValidityBits::ValidityBits(const Session &session, const vx_array *canonical) { vx_array_free(raw.array); throw_on_error(error); - bits_ = static_cast(vx_array_data_ptr_bool(owner_, &bit_offset_, &error)); + vx_bool_view view = vx_array_data_ptr_bool(owner_, &error); if (error != nullptr) { vx_array_free(owner_); } throw_on_error(error); + view_.bit_offset = view.bit_offset; + view_.bytes = {view.ptr, view.elements}; } ValidityBits::ValidityBits(ValidityBits &&other) noexcept - : owner_(other.owner_), bits_(other.bits_), bit_offset_(other.bit_offset_), - all_invalid_(other.all_invalid_) { + : owner_(other.owner_), view_(std::move(other.view_)), all_invalid_(other.all_invalid_) { other.owner_ = nullptr; - other.bits_ = nullptr; } ValidityBits &ValidityBits::operator=(ValidityBits &&other) noexcept { if (this != &other) { vx_array_free(owner_); owner_ = other.owner_; - bits_ = other.bits_; - bit_offset_ = other.bit_offset_; + view_ = std::move(other.view_); all_invalid_ = other.all_invalid_; other.owner_ = nullptr; - other.bits_ = nullptr; } return *this; } @@ -177,6 +174,22 @@ Array Array::primitive_raw(vx_ptype ptype, const void *data, size_t len, const V return Access::adopt(out); } +Array Array::bool_array(const BoolView &view, const Validity &validity) { + std::optional keep_alive; + vx_validity raw {}; + raw.type = static_cast(validity.type()); + if (validity.type() == ValidityType::FromArray) { + keep_alive = validity.array(); + raw.array = Access::c_ptr(*keep_alive); + } + + vx_error *error = nullptr; + vx_bool_view bool_view {view.bytes.data(), view.elements(), view.bit_offset}; + const vx_array *out = vx_array_new_bool(&bool_view, &raw, &error); + throw_on_error(error); + return Access::adopt(out); +} + Array Array::from_arrow(ArrowArray *array, ArrowSchema *schema, bool nullable) { vx_error *error = nullptr; const vx_array *out = vx_array_from_arrow(array, schema, nullable, &error); @@ -324,8 +337,11 @@ BytesView Array::bytes(const Session &session) const { return BytesView(std::move(canonical), std::move(validity), len); } -bool PrimitiveView::value(size_t i) const { - return vx_array_get_bool(Access::c_ptr(canonical_), i); +BoolView PrimitiveView::values() const { + vx_error *error = nullptr; + const vx_bool_view view = vx_array_data_ptr_bool(Access::c_ptr(canonical_), &error); + throw_on_error(error); + return {{view.ptr, vx_bool_view_len(view)}, view.bit_offset}; } std::string_view StringView::operator[](size_t i) const { diff --git a/lang/cpp/tests/array.cpp b/lang/cpp/tests/array.cpp index 998cb945833..0e8be9ad1b9 100644 --- a/lang/cpp/tests/array.cpp +++ b/lang/cpp/tests/array.cpp @@ -170,6 +170,46 @@ TEST_CASE("Slice", "[array]") { REQUIRE_THROWS_AS(a.slice(2, 100), VortexException); } +TEST_CASE("Bool array", "[array]") { + Session session; + + constexpr size_t ELEMENTS = 9; + std::array data = {true, false, true, true, false, true, true, false, true}; + std::vector bitpacked(2); + for (size_t i = 0; i < ELEMENTS; ++i) { + if (data[i]) { + bitpacked[i / 8] |= 1 << (i % 8); + } + } + + BoolView bool_view = {bitpacked, ELEMENTS}; + Array array = Array::bool_array(bool_view); + REQUIRE(array.size() == data.size()); + REQUIRE(array.has_dtype(DataTypeVariant::Bool)); + REQUIRE_FALSE(array.nullable()); + + auto view = array.bools(session); + BoolView values = view.values(); + for (size_t i = 0; i < values.elements(); ++i) { + REQUIRE(values[i] == data[i]); + } + + array = array.slice(3, array.size()); + view = array.bools(session); + values = view.values(); + + Array roundtrip = Array::bool_array(values); + REQUIRE(roundtrip.size() == view.size()); + REQUIRE_FALSE(roundtrip.nullable()); + + auto roundtrip_view = roundtrip.bools(session); + BoolView roundtrip_values = roundtrip_view.values(); + for (size_t i = 0; i < view.size(); ++i) { + REQUIRE(roundtrip_values[i] == values[i]); + REQUIRE(roundtrip_values[i] == data[i + 3]); + } +} + TEST_CASE("Error with a code", "[array]") { std::vector data = {0}; Array a = Array::primitive(data); diff --git a/lang/cpp/tests/expression.cpp b/lang/cpp/tests/expression.cpp index 1a55364df02..a8a5853bdcb 100644 --- a/lang/cpp/tests/expression.cpp +++ b/lang/cpp/tests/expression.cpp @@ -79,12 +79,13 @@ TEST_CASE("Operator overloading", "[expr]") { Array array = Array::primitive(data); Array applied = array.apply(expr::root() == expr::lit(2)); - auto bits = applied.bools(session); - REQUIRE(bits.size() == data.size()); - REQUIRE_FALSE(bits.value(0)); - REQUIRE(bits.value(1)); - REQUIRE(bits.value(2)); - REQUIRE_FALSE(bits.value(3)); + auto view = applied.bools(session); + REQUIRE(view.size() == data.size()); + BoolView values = view.values(); + REQUIRE_FALSE(values[0]); + REQUIRE(values[1]); + REQUIRE(values[2]); + REQUIRE_FALSE(values[3]); } TEST_CASE("Apply error", "[expr]") { diff --git a/vortex-ffi/cbindgen.toml b/vortex-ffi/cbindgen.toml index a0ee3aaed7d..2ed980567c9 100644 --- a/vortex-ffi/cbindgen.toml +++ b/vortex-ffi/cbindgen.toml @@ -63,6 +63,12 @@ typedef struct ArrowSchema FFI_ArrowSchema; typedef struct ArrowArray FFI_ArrowArray; typedef struct ArrowArrayStream FFI_ArrowArrayStream; #endif + +// Number of uint8_t words holding view's elements +#define vx_bool_view_len(X) (((X).elements + (X).bit_offset + 7) / 8) +// I'th element (bit) in view, LSB-first. Already accounts for bit_offset +#define vx_bool_view_nth(X, I) \ + ((((X).ptr[((I) + (X).bit_offset) / 8] & (1 << (((I) + (X).bit_offset) % 8))) != 0)) """ trailer = """ diff --git a/vortex-ffi/cinclude/vortex.h b/vortex-ffi/cinclude/vortex.h index b4c0039657e..aafed3b7357 100644 --- a/vortex-ffi/cinclude/vortex.h +++ b/vortex-ffi/cinclude/vortex.h @@ -56,6 +56,12 @@ typedef struct ArrowArray FFI_ArrowArray; typedef struct ArrowArrayStream FFI_ArrowArrayStream; #endif +// Number of uint8_t words holding view's elements +#define vx_bool_view_len(X) (((X).elements + (X).bit_offset + 7) / 8) +// I'th element (bit) in view, LSB-first. Already accounts for bit_offset +#define vx_bool_view_nth(X, I) \ + ((((X).ptr[((I) + (X).bit_offset) / 8] & (1 << (((I) + (X).bit_offset) % 8))) != 0)) + #include #include #include @@ -585,6 +591,39 @@ typedef struct { const vx_array *array; } vx_validity; +/** + * Readonly view over bitpacked booleans. + * + * "elements" is the number of bits/elements, not the number of uint8_t words. + * Bits are laid out LSB-first. + * + * "bit_offset" is in [0; 8) and lets a view start at a non-byte-aligned bit. + * You can use vx_bool_view_nth(view, index) macro to read a + * single element and vx_bool_view_len(view) to get the number of uint8_t words. + * + * Example: + * "view" holds 6 boolean elements starting at bit 2, first 5 set to "true", + * last is "false". + * + * uint8_t word = 0b01111100; + * vx_bool_view view = {&word, 6, 2}; + */ +typedef struct { + /** + * Element 0 is bit "bit_offset" of "ptr". + */ + const uint8_t *ptr; + /** + * Number of elements represented by "ptr". Get the number of uint8_t + * words in "ptr" with vx_bool_view_len(view). + */ + size_t elements; + /** + * Bit offset of element 0 within the first byte of "ptr". + */ + size_t bit_offset; +} vx_bool_view; + /** * A non owning view over a byte range. */ @@ -797,6 +836,25 @@ const vx_array *vx_array_new_primitive(vx_ptype ptype, const vx_validity *validity, vx_error **error); +/** + * Create a new Bool array from vx_bool_view. + * + * Example: + * + * a Bool array with 9 elements, first 8 are "true", last is "false". + * + * const vx_error* error = NULL; + * vx_validity validity = {}; + * validity.type = VX_VALIDITY_NON_NULLABLE; + * + * uint8_t words[2] = {0xff, 0}; // 11111111 00000000 + * vx_bool_view view = {words, 9, 0}; + * + * const vx_array* array = vx_array_new_bool(&view, &validity, &error); + * vx_array_free(array); + */ +const vx_array *vx_array_new_bool(const vx_bool_view *view, const vx_validity *validity, vx_error **error); + /** * Create a Vortex array by importing an Arrow array via the Arrow C Data Interface. * @@ -912,15 +970,25 @@ const vx_array *vx_array_canonicalize(const vx_session *session, const vx_array const void *vx_array_data_ptr_primitive(const vx_array *array, vx_error **error_out); /** - * Return a pointer to the bitpacked buffer of a canonical Bool array. - * Pointer is valid as long as "array" is valid. - * - * Writes bit offset of the first element into "bit_offset_out". - * "bit_offset_out" must not be NULL. + * Return vx_bool_view for a canonical Bool array. + * View is valid as long as "array" is valid. * * Errors if array is not a canonical Bool. + * + * Example: + * + * vx_validity validity = {}; + * validity.type = VX_VALIDITY_NON_NULLABLE; + * + * uint8_t words[2] = {0xff, 0}; // 11111111 00000000 + * vx_bool_view view = {words, 9, 0}; + * + * const vx_array* array = vx_array_new_bool(&view, &validity, &error); + * vx_bool_view other = vx_array_data_ptr_bool(array, &error); + * + * vx_array_free(array); */ -const void *vx_array_data_ptr_bool(const vx_array *array, size_t *bit_offset_out, vx_error **error_out); +vx_bool_view vx_array_data_ptr_bool(const vx_array *array, vx_error **error_out); /** * Apply the expression to the array, wrapping it with a ScalarFnArray. diff --git a/vortex-ffi/src/array.rs b/vortex-ffi/src/array.rs index 7c20bb72d46..388ad0a9a56 100644 --- a/vortex-ffi/src/array.rs +++ b/vortex-ffi/src/array.rs @@ -18,6 +18,7 @@ use vortex::array::Canonical; use vortex::array::IntoArray; use vortex::array::VortexSessionExecute; use vortex::array::arrays::Bool; +use vortex::array::arrays::BoolArray; use vortex::array::arrays::NullArray; use vortex::array::arrays::Primitive; use vortex::array::arrays::PrimitiveArray; @@ -27,6 +28,7 @@ use vortex::array::arrays::bool::BoolArrayExt; use vortex::array::arrays::struct_::StructArrayExt; use vortex::array::legacy_session; use vortex::array::validity::Validity; +use vortex::buffer::BitBuffer; use vortex::buffer::Buffer; use vortex::dtype::DType; use vortex::dtype::half::f16; @@ -70,6 +72,47 @@ arc_wrapper!( vx_array ); +/// Readonly view over bitpacked booleans. +/// +/// "elements" is the number of bits/elements, not the number of uint8_t words. +/// Bits are laid out LSB-first. +/// +/// "bit_offset" is in [0; 8) and lets a view start at a non-byte-aligned bit. +/// You can use vx_bool_view_nth(view, index) macro to read a +/// single element and vx_bool_view_len(view) to get the number of uint8_t words. +/// +/// Example: +/// "view" holds 6 boolean elements starting at bit 2, first 5 set to "true", +/// last is "false". +/// +/// uint8_t word = 0b01111100; +/// vx_bool_view view = {&word, 6, 2}; +#[repr(C)] +pub struct vx_bool_view { + /// Element 0 is bit "bit_offset" of "ptr". + pub ptr: *const u8, + /// Number of elements represented by "ptr". Get the number of uint8_t + /// words in "ptr" with vx_bool_view_len(view). + pub elements: usize, + /// Bit offset of element 0 within the first byte of "ptr". + pub bit_offset: usize, +} + +impl vx_bool_view { + /// {NULL, 0, 0} + const fn null() -> vx_bool_view { + vx_bool_view { + ptr: ptr::null(), + elements: 0, + bit_offset: 0, + } + } + + fn len(&self) -> usize { + (self.elements + self.bit_offset).div_ceil(8) + } +} + /// Borrow the [`ArrayRef`] behind a [`vx_array`] handle, erroring on a null pointer. /// /// A building block for FFI crates layered on top of the base Vortex C API. @@ -380,6 +423,47 @@ pub extern "C-unwind" fn vx_array_new_primitive( } } +/// Create a new Bool array from vx_bool_view. +/// +/// Example: +/// +/// a Bool array with 9 elements, first 8 are "true", last is "false". +/// +/// const vx_error* error = NULL; +/// vx_validity validity = {}; +/// validity.type = VX_VALIDITY_NON_NULLABLE; +/// +/// uint8_t words[2] = {0xff, 0}; // 11111111 00000000 +/// vx_bool_view view = {words, 9, 0}; +/// +/// const vx_array* array = vx_array_new_bool(&view, &validity, &error); +/// vx_array_free(array); +#[unsafe(no_mangle)] +pub extern "C-unwind" fn vx_array_new_bool( + view: *const vx_bool_view, + validity: *const vx_validity, + error: *mut *mut vx_error, +) -> *const vx_array { + try_or_default(error, || { + vortex_ensure!(!view.is_null()); + vortex_ensure!(!validity.is_null()); + let bits = unsafe { &*view }; + let validity = unsafe { &*validity }; + vortex_ensure!(bits.bit_offset < 8, "bit_offset must be in [0; 8)"); + let byte_len = bits.len(); + + let slice = if bits.ptr.is_null() { + unsafe { std::slice::from_raw_parts(NonNull::dangling().as_ptr(), byte_len) } + } else { + unsafe { std::slice::from_raw_parts(bits.ptr, byte_len) } + }; + let buffer = Buffer::copy_from(slice); + let bits = BitBuffer::new_with_offset(buffer, bits.elements, bits.bit_offset); + let array = BoolArray::try_new(bits, validity.into())?; + Ok(vx_array::new(Arc::new(array.into_array()))) + }) +} + /// Create a Vortex array by importing an Arrow array via the Arrow C Data Interface. /// /// `array` and `schema` together describe a single Arrow array (the standard Arrow C Data @@ -591,31 +675,43 @@ pub unsafe extern "C-unwind" fn vx_array_data_ptr_primitive( }) } -/// Return a pointer to the bitpacked buffer of a canonical Bool array. -/// Pointer is valid as long as "array" is valid. -/// -/// Writes bit offset of the first element into "bit_offset_out". -/// "bit_offset_out" must not be NULL. +/// Return vx_bool_view for a canonical Bool array. +/// View is valid as long as "array" is valid. /// /// Errors if array is not a canonical Bool. +/// +/// Example: +/// +/// vx_validity validity = {}; +/// validity.type = VX_VALIDITY_NON_NULLABLE; +/// +/// uint8_t words[2] = {0xff, 0}; // 11111111 00000000 +/// vx_bool_view view = {words, 9, 0}; +/// +/// const vx_array* array = vx_array_new_bool(&view, &validity, &error); +/// vx_bool_view other = vx_array_data_ptr_bool(array, &error); +/// +/// vx_array_free(array); #[unsafe(no_mangle)] pub unsafe extern "C-unwind" fn vx_array_data_ptr_bool( array: *const vx_array, - bit_offset_out: *mut usize, error_out: *mut *mut vx_error, -) -> *const c_void { - try_or(error_out, ptr::null(), || { +) -> vx_bool_view { + try_or(error_out, vx_bool_view::null(), || { let array = unsafe { vx_array_ref(array) }?; - vortex_ensure!(!bit_offset_out.is_null(), "null bit_offset_out"); - let bool_array = array.as_opt::().ok_or_else(|| { - vortex_err!( - "vx_array_data_ptr_bool requires a canonical Bool array, got {}", - array.encoding_id() - ) + let array = array.as_opt::().ok_or_else(|| { + let id = array.encoding_id(); + vortex_err!("vx_array_data_ptr_bool requires a canonical Bool array, got {id}") })?; - let bits = bool_array.to_bit_buffer(); - unsafe { bit_offset_out.write(bits.offset()) }; - Ok(bits.inner().as_ptr().cast()) + let bits = array.to_bit_buffer(); + let ptr = bits.inner().as_ptr().cast(); + let elements = bits.len(); + let bit_offset = bits.offset(); + Ok(vx_bool_view { + ptr, + elements, + bit_offset, + }) }) } @@ -1092,7 +1188,6 @@ mod tests { unsafe { let session = vx_session_new(); let mut error = ptr::null_mut(); - let mut bit_offset = usize::MAX; let array = vx_array::new(Arc::new(primitive.into_array())); let canonical = vx_array_canonicalize(session, array, &raw mut error); @@ -1114,12 +1209,10 @@ mod tests { )); let validity_bools = vx_array_canonicalize(session, validity.array, &raw mut error); assert_no_error(error); - let bits = vx_array_data_ptr_bool(validity_bools, &raw mut bit_offset, &raw mut error); + let view = vx_array_data_ptr_bool(validity_bools, &raw mut error); assert_no_error(error); - assert_eq!( - *bits.cast::().add(bit_offset / 8) >> (bit_offset % 8) & 0b111, - 0b101 - ); + let byte = *view.ptr.add(view.bit_offset / 8); + assert_eq!((byte >> (view.bit_offset % 8)) & 0b111, 0b101); vx_array_free(validity_bools); vx_array_free(validity.array); @@ -1156,6 +1249,126 @@ mod tests { } } + #[test] + #[cfg_attr(miri, ignore)] + fn test_bool() { + unsafe { + let words: [u8; 2] = [u8::MAX, 0]; + let validity = vx_validity { + r#type: vx_validity_type::VX_VALIDITY_NON_NULLABLE, + array: ptr::null(), + }; + + let view = vx_bool_view { + ptr: words.as_ptr(), + elements: 9, + bit_offset: 0, + }; + + let mut error = ptr::null_mut(); + let array = vx_array_new_bool(&raw const view, &raw const validity, &raw mut error); + assert_no_error(error); + assert!(!array.is_null()); + assert!(vx_array_has_dtype(array, vx_dtype_variant::DTYPE_BOOL)); + assert_eq!(vx_array_len(array), 9); + + for i in 0..8 { + assert!(vx_array_get_bool(array, i)); + } + assert!(!vx_array_get_bool(array, 8)); + + let other_view = vx_array_data_ptr_bool(array, &raw mut error); + assert_no_error(error); + assert_eq!(other_view.elements, 9); + assert_eq!(other_view.bit_offset, 0); + assert_eq!(*other_view.ptr, words[0]); + + vx_array_free(array); + } + } + + #[test] + #[cfg_attr(miri, ignore)] + fn test_bool_offset() { + // 6 elements starting at bit 2, first 5 true, last false. + let word: u8 = 0b01111100; + + unsafe { + let validity = vx_validity { + r#type: vx_validity_type::VX_VALIDITY_NON_NULLABLE, + array: ptr::null(), + }; + let view = vx_bool_view { + ptr: &raw const word, + elements: 6, + bit_offset: 2, + }; + + let mut error = ptr::null_mut(); + let array = vx_array_new_bool(&raw const view, &raw const validity, &raw mut error); + assert_no_error(error); + assert_eq!(vx_array_len(array), 6); + + for i in 0..5 { + assert!(vx_array_get_bool(array, i), "index {i}"); + } + assert!(!vx_array_get_bool(array, 5)); + + vx_array_free(array); + } + } + + #[test] + #[cfg_attr(miri, ignore)] + fn test_bool_roundtrip() { + let expected = [ + true, true, false, true, false, true, true, false, true, true, + ]; + + unsafe { + let session = vx_session_new(); + let mut error = ptr::null_mut(); + let validity = vx_validity { + r#type: vx_validity_type::VX_VALIDITY_NON_NULLABLE, + array: ptr::null(), + }; + + let mut words = vec![0u8; expected.len().div_ceil(8)]; + for (i, value) in expected.iter().enumerate() { + if *value { + words[i / 8] |= 1 << (i % 8); + } + } + + let view = vx_bool_view { + ptr: words.as_ptr(), + elements: expected.len(), + bit_offset: 0, + }; + let array = vx_array_new_bool(&raw const view, &raw const validity, &raw mut error); + assert_no_error(error); + + let canonical = vx_array_canonicalize(session, array, &raw mut error); + assert_no_error(error); + + for (i, value) in expected.iter().enumerate() { + assert_eq!(vx_array_get_bool(canonical, i), *value, "index {i}"); + } + + let bits = vx_array_data_ptr_bool(canonical, &raw mut error); + assert_no_error(error); + for (i, value) in expected.iter().enumerate() { + let bit = bits.bit_offset + i; + let actual = (*bits.ptr.add(bit / 8) >> (bit % 8)) & 1 == 1; + assert_eq!(actual, *value, "index {i}"); + } + + vx_array_free(canonical); + vx_array_free(array); + vx_session_free(session); + } + } + #[test] #[cfg_attr(miri, ignore)] fn test_data_ptr_bool() { @@ -1166,7 +1379,6 @@ mod tests { unsafe { let session = vx_session_new(); let mut error = ptr::null_mut(); - let mut bit_offset = usize::MAX; let array = vx_array::new(Arc::new(bools.into_array())); let sliced = vx_array_slice(array, 3, 10, &raw mut error); @@ -1175,15 +1387,16 @@ mod tests { let canonical = vx_array_canonicalize(session, sliced, &raw mut error); assert_no_error(error); - let bits = vx_array_data_ptr_bool(canonical, &raw mut bit_offset, &raw mut error); + let bits = vx_array_data_ptr_bool(canonical, &raw mut error); assert_no_error(error); - assert!(bit_offset < 8); + assert!(bits.bit_offset < 8); + assert_eq!(bits.elements, 7); for (i, expected) in [true, false, true, true, false, true, true] .into_iter() .enumerate() { - let bit = bit_offset + i; - let actual = (*bits.cast::().add(bit / 8) >> (bit % 8)) & 1 == 1; + let bit = bits.bit_offset + i; + let actual = (*bits.ptr.add(bit / 8) >> (bit % 8)) & 1 == 1; assert_eq!(actual, expected, "bit {i}"); } vx_array_free(canonical); @@ -1207,9 +1420,8 @@ mod tests { assert!(data.is_null()); assert_error(error); - let mut bit_offset = usize::MAX; - let bits = vx_array_data_ptr_bool(array, &raw mut bit_offset, &raw mut error); - assert!(bits.is_null()); + let bits = vx_array_data_ptr_bool(array, &raw mut error); + assert!(bits.ptr.is_null()); assert_error(error); vx_array_free(array); diff --git a/vortex-ffi/test/array.cpp b/vortex-ffi/test/array.cpp index f8db2661b8d..d6189c38194 100644 --- a/vortex-ffi/test/array.cpp +++ b/vortex-ffi/test/array.cpp @@ -1,6 +1,8 @@ // SPDX-License-Identifier: Apache-2.0 // SPDX-FileCopyrightText: Copyright the Vortex contributors #include +#include +#include #include #include "common.h" @@ -51,6 +53,109 @@ TEST_CASE("Primitive array creation", "[array]") { vx_array_free(array); } +TEST_CASE("Bool view", "[array]") { + std::vector buffer(2, UINT8_MAX); + constexpr size_t ELEMENTS = 10; + constexpr size_t OFFSET = 6; + vx_bool_view view {.ptr = buffer.data(), .elements = ELEMENTS, .bit_offset = OFFSET}; + + REQUIRE(vx_bool_view_len(view) == 2); + + for (size_t i = 0; i < view.elements; ++i) { + REQUIRE(vx_bool_view_nth(view, i) == 1); + } + + // bit_offset is 6. buffer[0] holds elements 0-1. buffer[1] holds elements + // 2-9. buffer[1] = 1 sets element 2 to 1 and elements 3-9 to 0. + buffer[1] = 1; + REQUIRE(vx_bool_view_nth(view, 0) == 1); + REQUIRE(vx_bool_view_nth(view, 1) == 1); + REQUIRE(vx_bool_view_nth(view, 2) == 1); + for (size_t i = 3; i < ELEMENTS; ++i) { + REQUIRE(vx_bool_view_nth(view, i) == 0); + } +} + +TEST_CASE("Bool array", "[array]") { + const std::vector values = {true, true, false, true, false, true, true, false, true, true}; + std::vector words(2, 0); + for (size_t i = 0; i < values.size(); ++i) { + if (values[i]) { + words[i / 8] |= static_cast(1u << (i % 8)); + } + } + vx_bool_view view {.ptr = words.data(), .elements = values.size(), .bit_offset = 0}; + + vx_validity validity = {}; + validity.type = VX_VALIDITY_NON_NULLABLE; + vx_error *error = nullptr; + const vx_array *array = vx_array_new_bool(&view, &validity, &error); + require_no_error(error); + defer { + vx_array_free(array); + }; + REQUIRE(vx_array_len(array) == values.size()); + + for (size_t i = 0; i < values.size(); ++i) { + REQUIRE(vx_array_get_bool(array, i) == values[i]); + } + + view = vx_array_data_ptr_bool(array, &error); + require_no_error(error); + for (size_t i = 0; i < values.size(); ++i) { + const size_t bit = view.bit_offset + i; + const bool actual = (view.ptr[bit / 8] >> (bit % 8)) & 1; + REQUIRE(actual == values[i]); + } +} + +TEST_CASE("Array with validity", "[array]") { + const std::vector valid = {true, false, true, true, false}; + std::vector words(1, 0); + for (size_t i = 0; i < valid.size(); ++i) { + if (valid[i]) { + words[0] |= static_cast(1u << i); + } + } + vx_bool_view validity_view {.ptr = words.data(), .elements = valid.size(), .bit_offset = 0}; + + vx_validity validity = {}; + validity.type = VX_VALIDITY_NON_NULLABLE; + vx_error *error = nullptr; + const vx_array *validity_array = vx_array_new_bool(&validity_view, &validity, &error); + require_no_error(error); + defer { + vx_array_free(validity_array); + }; + + validity = {}; + validity.type = VX_VALIDITY_ARRAY; + validity.array = validity_array; + + std::vector data(valid.size(), 7); + const vx_array *array = vx_array_new_primitive(PTYPE_U8, data.data(), data.size(), &validity, &error); + require_no_error(error); + defer { + vx_array_free(array); + }; + + REQUIRE(vx_array_is_nullable(array)); + REQUIRE(vx_array_len(array) == valid.size()); + + for (size_t i = 0; i < valid.size(); ++i) { + REQUIRE(vx_array_element_is_invalid(array, i, &error) == !valid[i]); + require_no_error(error); + } + REQUIRE(vx_array_invalid_count(array, &error) == 2); + require_no_error(error); + + validity = {}; + vx_array_get_validity(array, &validity, &error); + require_no_error(error); + REQUIRE(validity.type == VX_VALIDITY_ARRAY); + vx_array_free(validity.array); +} + TEST_CASE("Struct array creation", "[array]") { vx_error *error = nullptr; From 3cef8db773f384d2aa0c387e573f392a5833cd5e Mon Sep 17 00:00:00 2001 From: Mikhail Kot Date: Thu, 23 Jul 2026 16:48:57 +0100 Subject: [PATCH 2/2] fix --- lang/cpp/examples/CMakeLists.txt | 2 +- lang/cpp/examples/reader.cpp | 1 - lang/cpp/examples/validity.cpp | 45 +++++++++++++++++++++++++++++++ lang/cpp/include/vortex/array.hpp | 23 +++++++++------- vortex-ffi/cbindgen.toml | 2 +- vortex-ffi/test/array.cpp | 2 +- 6 files changed, 61 insertions(+), 14 deletions(-) create mode 100644 lang/cpp/examples/validity.cpp diff --git a/lang/cpp/examples/CMakeLists.txt b/lang/cpp/examples/CMakeLists.txt index 2733b745b96..43face3e788 100644 --- a/lang/cpp/examples/CMakeLists.txt +++ b/lang/cpp/examples/CMakeLists.txt @@ -1,7 +1,7 @@ # SPDX-License-Identifier: Apache-2.0 # SPDX-FileCopyrightText: Copyright the Vortex contributors -foreach(name reader writer dtype scan scan_to_arrow) +foreach(name reader writer validity dtype scan scan_to_arrow) add_executable(${name} ${name}.cpp) target_link_libraries(${name} PRIVATE vortex_cxx_shared nanoarrow_shared) endforeach() diff --git a/lang/cpp/examples/reader.cpp b/lang/cpp/examples/reader.cpp index 48656c046f7..33231a873b7 100644 --- a/lang/cpp/examples/reader.cpp +++ b/lang/cpp/examples/reader.cpp @@ -10,7 +10,6 @@ using namespace std::string_view_literals; using namespace vortex; using namespace expr; using namespace ops; // overloaded >= for Expressions -namespace fs = std::filesystem; int main() { const Session session; diff --git a/lang/cpp/examples/validity.cpp b/lang/cpp/examples/validity.cpp new file mode 100644 index 00000000000..a38ce2f935a --- /dev/null +++ b/lang/cpp/examples/validity.cpp @@ -0,0 +1,45 @@ +// SPDX-License-Identifier: Apache-2.0 +// SPDX-FileCopyrightText: Copyright the Vortex contributors + +#include "vortex/dtype.hpp" +#include + +#include +#include +#include + +using namespace std::string_view_literals; +using namespace vortex; +using dtype::Nullable; + +std::vector bitpack(const std::vector &validity) { + std::vector out; + + return out; +} + +int main() { + const std::string_view name = "validity.vortex"; + + const Session session; + const DataType dtype = dtype::int64(Nullable); + Writer writer = Writer::open(session, name, dtype); + writer.push(array); + writer.finish(); + + const DataSource ds = DataSource::open(session, {name}); + Scan scan = ds.scan(); + for (Partition &partition : scan.partitions()) { + for (Array &array : partition.batches()) { + const Array age = array.field("age"); + const PrimitiveView age_view = age.values(session); + const std::span age_values = age_view.values(); + for (uint8_t value : age_values) { + std::cout << int(value) << " "; + } + } + } + std::cout << "\n"; + + return 0; +} diff --git a/lang/cpp/include/vortex/array.hpp b/lang/cpp/include/vortex/array.hpp index 28edcc3e323..05bff4fc940 100644 --- a/lang/cpp/include/vortex/array.hpp +++ b/lang/cpp/include/vortex/array.hpp @@ -32,7 +32,7 @@ class StringView; class BytesView; /** - * Readonly view over bitpacked booleans. + * Non-owning view over bitpacked booleans. * * Bits are laid out LSB-first. * "bit_offset" is in [0; 8) and lets a view start at a non-byte-aligned bit. @@ -41,30 +41,33 @@ class BytesView; * "view" holds 6 boolean elements starting at bit 2, first 5 set to "true", * last is "false". * + * // [6][5][4][3][2][1][0][offset bit][offset bit] * uint8_t word = 0b01111100; * BoolView view = {&word, 6, 2}; */ struct BoolView { - std::span bytes; + const uint8_t* ptr = nullptr; + + /* + * Number of elements (bits) in the view. This is not the number of uint8_t + * words in "ptr", use words() for that. + */ + size_t elements = 0; + /* * Bit offset of first element in "bytes". * Example: if bit_offset is 2, first bit is at bytes[0] & (1 << 2). */ size_t bit_offset = 0; - // Get bit at "index". Index is in [0; elements()). + // Get bit at "index". Index is in [0; elements). inline bool operator[](size_t index) const { - return (bytes.data()[(index + bit_offset) / 8] & (1 << ((index + bit_offset) % 8))); + return vx_bool_view_nth(*this, index); } // Number of uint8_t words storing elements. inline size_t words() const { - return bytes.size(); - } - - // Number of set elements/bits - inline size_t elements() const { - return words() * 8 - bit_offset; + return vx_bool_view_words(*this); } }; diff --git a/vortex-ffi/cbindgen.toml b/vortex-ffi/cbindgen.toml index 2ed980567c9..f10f0268f5d 100644 --- a/vortex-ffi/cbindgen.toml +++ b/vortex-ffi/cbindgen.toml @@ -65,7 +65,7 @@ typedef struct ArrowArrayStream FFI_ArrowArrayStream; #endif // Number of uint8_t words holding view's elements -#define vx_bool_view_len(X) (((X).elements + (X).bit_offset + 7) / 8) +#define vx_bool_view_words(X) (((X).elements + (X).bit_offset + 7) / 8) // I'th element (bit) in view, LSB-first. Already accounts for bit_offset #define vx_bool_view_nth(X, I) \ ((((X).ptr[((I) + (X).bit_offset) / 8] & (1 << (((I) + (X).bit_offset) % 8))) != 0)) diff --git a/vortex-ffi/test/array.cpp b/vortex-ffi/test/array.cpp index d6189c38194..34427649f11 100644 --- a/vortex-ffi/test/array.cpp +++ b/vortex-ffi/test/array.cpp @@ -59,7 +59,7 @@ TEST_CASE("Bool view", "[array]") { constexpr size_t OFFSET = 6; vx_bool_view view {.ptr = buffer.data(), .elements = ELEMENTS, .bit_offset = OFFSET}; - REQUIRE(vx_bool_view_len(view) == 2); + REQUIRE(vx_bool_view_words(view) == 2); for (size_t i = 0; i < view.elements; ++i) { REQUIRE(vx_bool_view_nth(view, i) == 1);