From 5279d337c51f95c9581fcc31f44e5f05006af5bc Mon Sep 17 00:00:00 2001 From: Mikhail Kot Date: Wed, 22 Jul 2026 16:22:02 +0100 Subject: [PATCH 1/4] fix --- lang/cpp/include/vortex/array.hpp | 64 ++++++- lang/cpp/src/array.cpp | 47 +++-- lang/cpp/tests/array.cpp | 40 +++++ lang/cpp/tests/expression.cpp | 14 +- vortex-ffi/cbindgen.toml | 6 + vortex-ffi/cinclude/vortex.h | 79 ++++++++- vortex-ffi/src/array.rs | 275 ++++++++++++++++++++++++++---- vortex-ffi/test/array.cpp | 105 ++++++++++++ 8 files changed, 560 insertions(+), 70 deletions(-) diff --git a/lang/cpp/include/vortex/array.hpp b/lang/cpp/include/vortex/array.hpp index 3c0df8865c7..c60dde2a8bd 100644 --- a/lang/cpp/include/vortex/array.hpp +++ b/lang/cpp/include/vortex/array.hpp @@ -12,6 +12,7 @@ #include #include +#include #include #include #include @@ -31,6 +32,45 @@ 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 { + 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()). + inline bool operator[](size_t index) const { + return vx_bool_view_nth(*this, index); + } + + // Number of uint8_t words storing elements. + inline size_t words() const { + return vx_bool_view_words(*this); + } +}; + /* * Validity type tells us whether there are null/invalid values in an Array. */ @@ -102,8 +142,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 @@ -132,6 +171,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. @@ -296,17 +346,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 d68d8729380..5d509f2a892 100644 --- a/lang/cpp/src/array.cpp +++ b/lang/cpp/src/array.cpp @@ -78,11 +78,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) { @@ -106,29 +105,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; } @@ -179,7 +176,23 @@ Array Array::primitive_raw(vx_ptype ptype, const void *data, size_t len, const V return Access::adopt(out); } -Array Array::from_arrow(const Session &session, ArrowArray *array, ArrowSchema *schema, bool nullable) { +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(const Session& session, ArrowArray *array, ArrowSchema *schema, bool nullable) { vx_error *error = nullptr; const vx_array *out = vx_array_from_arrow(Access::c_ptr(session), array, schema, nullable, &error); throw_on_error(error); @@ -333,13 +346,11 @@ Scalar Array::scalar_at(const Session &session, size_t index) const { return Access::adopt(scalar); } -bool PrimitiveView::value(size_t i) const { - if (i >= size_) { - throw VortexException("index " + std::to_string(i) + " out of bounds for view of size " + - std::to_string(size_), - ErrorCode::OutOfBounds); - } - 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 8145ab6b6d4..01789eb3254 100644 --- a/lang/cpp/tests/expression.cpp +++ b/lang/cpp/tests/expression.cpp @@ -79,13 +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)); - REQUIRE_THROWS_AS(bits.value(data.size()), VortexException); + PrimitiveView 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 836c8ada00b..59d2ff46336 100644 --- a/vortex-ffi/cbindgen.toml +++ b/vortex-ffi/cbindgen.toml @@ -67,6 +67,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_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)) """ trailer = """ diff --git a/vortex-ffi/cinclude/vortex.h b/vortex-ffi/cinclude/vortex.h index c04d2da796d..c10d6102e55 100644 --- a/vortex-ffi/cinclude/vortex.h +++ b/vortex-ffi/cinclude/vortex.h @@ -60,6 +60,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_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)) + #include #include #include @@ -475,6 +481,38 @@ typedef struct { const vx_array *array; } vx_validity; +/** + * Readonly view over bitpacked booleans. + * + * "elements" is the number of bits/elements. Use vx_bool_view_words(view) to + * get 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. + * Use vx_bool_view_nth(view, index) macro to read a single element. + * + * Example: + * "view" holds 6 boolean elements, bit_offset=2, first 5 elements are "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". + */ + 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. */ @@ -690,6 +728,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. * @@ -778,15 +835,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 3d4b9758567..f688f47f6ef 100644 --- a/vortex-ffi/src/array.rs +++ b/vortex-ffi/src/array.rs @@ -16,6 +16,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; @@ -25,6 +26,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; @@ -68,6 +70,46 @@ box_wrapper!( vx_array ); +/// Readonly view over bitpacked booleans. +/// +/// "elements" is the number of bits/elements. Use vx_bool_view_words(view) to +/// get 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. +/// Use vx_bool_view_nth(view, index) macro to read a single element. +/// +/// Example: +/// "view" holds 6 boolean elements, bit_offset=2, first 5 elements are "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". + 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. @@ -383,6 +425,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(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 @@ -573,31 +656,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(), || { - let array = vx_array::as_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() - ) +) -> vx_bool_view { + try_or(error_out, vx_bool_view::null(), || { + let array = unsafe { vx_array_ref(array) }?; + 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, + }) }) } @@ -1031,7 +1126,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(primitive.into_array()); let canonical = vx_array_canonicalize(session, array, &raw mut error); @@ -1053,12 +1147,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); @@ -1095,6 +1187,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() { @@ -1105,7 +1317,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(bools.into_array()); let sliced = vx_array_slice(array, 3, 10, &raw mut error); @@ -1114,15 +1325,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); @@ -1146,9 +1358,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 3a90fe909c2..48cf9bdb6b7 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" @@ -56,6 +58,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 293a9f0187a09bce441b1fc208c3a7317a2c353e Mon Sep 17 00:00:00 2001 From: Mikhail Kot Date: Fri, 11 Sep 2026 15:41:12 +0100 Subject: [PATCH 2/4] better Signed-off-by: Mikhail Kot --- lang/cpp/src/array.cpp | 11 ++++++----- lang/cpp/tests/array.cpp | 4 ++-- vortex-ffi/src/array.rs | 3 ++- vortex-ffi/test/array.cpp | 9 +++++++-- 4 files changed, 17 insertions(+), 10 deletions(-) diff --git a/lang/cpp/src/array.cpp b/lang/cpp/src/array.cpp index 5d509f2a892..7364c9abc34 100644 --- a/lang/cpp/src/array.cpp +++ b/lang/cpp/src/array.cpp @@ -81,7 +81,7 @@ bool ValidityBits::is_null(size_t index) const { if (owner_ == nullptr) { return false; } - return view_[index]; + return !view_[index]; } ValidityBits::ValidityBits(const Session &session, const vx_array *canonical) { @@ -111,7 +111,8 @@ ValidityBits::ValidityBits(const Session &session, const vx_array *canonical) { } throw_on_error(error); view_.bit_offset = view.bit_offset; - view_.bytes = {view.ptr, view.elements}; + view_.ptr = view.ptr; + view_.elements = view.elements; } ValidityBits::ValidityBits(ValidityBits &&other) noexcept @@ -186,13 +187,13 @@ Array Array::bool_array(const BoolView &view, const Validity &validity) { } vx_error *error = nullptr; - vx_bool_view bool_view {view.bytes.data(), view.elements(), view.bit_offset}; + vx_bool_view bool_view {view.ptr, 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(const Session& session, ArrowArray *array, ArrowSchema *schema, bool nullable) { +Array Array::from_arrow(const Session &session, ArrowArray *array, ArrowSchema *schema, bool nullable) { vx_error *error = nullptr; const vx_array *out = vx_array_from_arrow(Access::c_ptr(session), array, schema, nullable, &error); throw_on_error(error); @@ -350,7 +351,7 @@ 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}; + return {view.ptr, view.elements, 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 0e8be9ad1b9..8e12bd6ccb2 100644 --- a/lang/cpp/tests/array.cpp +++ b/lang/cpp/tests/array.cpp @@ -182,7 +182,7 @@ TEST_CASE("Bool array", "[array]") { } } - BoolView bool_view = {bitpacked, ELEMENTS}; + BoolView bool_view = {bitpacked.data(), ELEMENTS, 0}; Array array = Array::bool_array(bool_view); REQUIRE(array.size() == data.size()); REQUIRE(array.has_dtype(DataTypeVariant::Bool)); @@ -190,7 +190,7 @@ TEST_CASE("Bool array", "[array]") { auto view = array.bools(session); BoolView values = view.values(); - for (size_t i = 0; i < values.elements(); ++i) { + for (size_t i = 0; i < values.elements; ++i) { REQUIRE(values[i] == data[i]); } diff --git a/vortex-ffi/src/array.rs b/vortex-ffi/src/array.rs index f688f47f6ef..6b15ed834e1 100644 --- a/vortex-ffi/src/array.rs +++ b/vortex-ffi/src/array.rs @@ -455,7 +455,8 @@ pub extern "C-unwind" fn vx_array_new_bool( let byte_len = bits.len(); let slice = if bits.ptr.is_null() { - unsafe { std::slice::from_raw_parts(NonNull::dangling().as_ptr(), byte_len) } + vortex_ensure!(byte_len == 0, "nonzero length but null pointer for view"); + &[] } else { unsafe { std::slice::from_raw_parts(bits.ptr, byte_len) } }; diff --git a/vortex-ffi/test/array.cpp b/vortex-ffi/test/array.cpp index 48cf9bdb6b7..dc409e18c24 100644 --- a/vortex-ffi/test/array.cpp +++ b/vortex-ffi/test/array.cpp @@ -64,7 +64,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); @@ -115,6 +115,11 @@ TEST_CASE("Bool array", "[array]") { } TEST_CASE("Array with validity", "[array]") { + vx_session *session = vx_session_new(); + defer { + vx_session_free(session); + }; + const std::vector valid = {true, false, true, true, false}; std::vector words(1, 0); for (size_t i = 0; i < valid.size(); ++i) { @@ -148,7 +153,7 @@ TEST_CASE("Array with validity", "[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(vx_array_element_is_invalid(session, array, i, &error) == !valid[i]); require_no_error(error); } REQUIRE(vx_array_invalid_count(array, &error) == 2); From 2ebbb543147a0f1f84e2aa88affb7d43fc180250 Mon Sep 17 00:00:00 2001 From: Mikhail Kot Date: Fri, 11 Sep 2026 15:43:05 +0100 Subject: [PATCH 3/4] comment Signed-off-by: Mikhail Kot --- vortex-ffi/src/array.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vortex-ffi/src/array.rs b/vortex-ffi/src/array.rs index 6b15ed834e1..e708795b927 100644 --- a/vortex-ffi/src/array.rs +++ b/vortex-ffi/src/array.rs @@ -435,7 +435,7 @@ pub extern "C-unwind" fn vx_array_new_primitive( /// vx_validity validity = {}; /// validity.type = VX_VALIDITY_NON_NULLABLE; /// -/// uint8_t words[2] = {0xff, 0}; // 11111111 00000000 +/// 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); @@ -667,7 +667,7 @@ pub unsafe extern "C-unwind" fn vx_array_data_ptr_primitive( /// vx_validity validity = {}; /// validity.type = VX_VALIDITY_NON_NULLABLE; /// -/// uint8_t words[2] = {0xff, 0}; // 11111111 00000000 +/// 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); From f72d191f8538eece1851d45c6baedd55fac07eed Mon Sep 17 00:00:00 2001 From: Mikhail Kot Date: Fri, 11 Sep 2026 15:48:20 +0100 Subject: [PATCH 4/4] ffi Signed-off-by: Mikhail Kot --- vortex-ffi/cinclude/vortex.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vortex-ffi/cinclude/vortex.h b/vortex-ffi/cinclude/vortex.h index c10d6102e55..fa214e8efbe 100644 --- a/vortex-ffi/cinclude/vortex.h +++ b/vortex-ffi/cinclude/vortex.h @@ -739,7 +739,7 @@ const vx_array *vx_array_new_primitive(vx_ptype ptype, * vx_validity validity = {}; * validity.type = VX_VALIDITY_NON_NULLABLE; * - * uint8_t words[2] = {0xff, 0}; // 11111111 00000000 + * 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); @@ -845,7 +845,7 @@ const void *vx_array_data_ptr_primitive(const vx_array *array, vx_error **error_ * vx_validity validity = {}; * validity.type = VX_VALIDITY_NON_NULLABLE; * - * uint8_t words[2] = {0xff, 0}; // 11111111 00000000 + * 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);