diff --git a/vortex-array/public-api.lock b/vortex-array/public-api.lock index b388e7e3a21..8ae349c3da5 100644 --- a/vortex-array/public-api.lock +++ b/vortex-array/public-api.lock @@ -1494,7 +1494,7 @@ pub fn vortex_array::arrays::Constant::child_name(array: vortex_array::ArrayView pub fn vortex_array::arrays::Constant::deserialize(&self, dtype: &vortex_array::dtype::DType, len: usize, _metadata: &[u8], buffers: &[vortex_array::buffer::BufferHandle], _children: &dyn vortex_array::serde::ArrayChildren, session: &vortex_session::VortexSession) -> vortex_error::VortexResult> -pub fn vortex_array::arrays::Constant::execute(array: vortex_array::Array, _ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult +pub fn vortex_array::arrays::Constant::execute(array: vortex_array::Array, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult pub fn vortex_array::arrays::Constant::execute_parent(array: vortex_array::ArrayView<'_, Self>, parent: &vortex_array::ArrayRef, child_idx: usize, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult> @@ -5312,7 +5312,7 @@ pub fn vortex_array::arrays::Constant::child_name(array: vortex_array::ArrayView pub fn vortex_array::arrays::Constant::deserialize(&self, dtype: &vortex_array::dtype::DType, len: usize, _metadata: &[u8], buffers: &[vortex_array::buffer::BufferHandle], _children: &dyn vortex_array::serde::ArrayChildren, session: &vortex_session::VortexSession) -> vortex_error::VortexResult> -pub fn vortex_array::arrays::Constant::execute(array: vortex_array::Array, _ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult +pub fn vortex_array::arrays::Constant::execute(array: vortex_array::Array, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult pub fn vortex_array::arrays::Constant::execute_parent(array: vortex_array::ArrayView<'_, Self>, parent: &vortex_array::ArrayRef, child_idx: usize, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult> @@ -19404,7 +19404,7 @@ pub fn vortex_array::arrays::Constant::child_name(array: vortex_array::ArrayView pub fn vortex_array::arrays::Constant::deserialize(&self, dtype: &vortex_array::dtype::DType, len: usize, _metadata: &[u8], buffers: &[vortex_array::buffer::BufferHandle], _children: &dyn vortex_array::serde::ArrayChildren, session: &vortex_session::VortexSession) -> vortex_error::VortexResult> -pub fn vortex_array::arrays::Constant::execute(array: vortex_array::Array, _ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult +pub fn vortex_array::arrays::Constant::execute(array: vortex_array::Array, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult pub fn vortex_array::arrays::Constant::execute_parent(array: vortex_array::ArrayView<'_, Self>, parent: &vortex_array::ArrayRef, child_idx: usize, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult> @@ -20376,7 +20376,7 @@ pub fn vortex_array::arrays::Constant::child_name(array: vortex_array::ArrayView pub fn vortex_array::arrays::Constant::deserialize(&self, dtype: &vortex_array::dtype::DType, len: usize, _metadata: &[u8], buffers: &[vortex_array::buffer::BufferHandle], _children: &dyn vortex_array::serde::ArrayChildren, session: &vortex_session::VortexSession) -> vortex_error::VortexResult> -pub fn vortex_array::arrays::Constant::execute(array: vortex_array::Array, _ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult +pub fn vortex_array::arrays::Constant::execute(array: vortex_array::Array, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult pub fn vortex_array::arrays::Constant::execute_parent(array: vortex_array::ArrayView<'_, Self>, parent: &vortex_array::ArrayRef, child_idx: usize, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult> @@ -23070,7 +23070,7 @@ pub fn vortex_array::arrays::Constant::child_name(array: vortex_array::ArrayView pub fn vortex_array::arrays::Constant::deserialize(&self, dtype: &vortex_array::dtype::DType, len: usize, _metadata: &[u8], buffers: &[vortex_array::buffer::BufferHandle], _children: &dyn vortex_array::serde::ArrayChildren, session: &vortex_session::VortexSession) -> vortex_error::VortexResult> -pub fn vortex_array::arrays::Constant::execute(array: vortex_array::Array, _ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult +pub fn vortex_array::arrays::Constant::execute(array: vortex_array::Array, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult pub fn vortex_array::arrays::Constant::execute_parent(array: vortex_array::ArrayView<'_, Self>, parent: &vortex_array::ArrayRef, child_idx: usize, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult> @@ -24290,7 +24290,7 @@ pub fn vortex_array::arrays::Constant::child_name(array: vortex_array::ArrayView pub fn vortex_array::arrays::Constant::deserialize(&self, dtype: &vortex_array::dtype::DType, len: usize, _metadata: &[u8], buffers: &[vortex_array::buffer::BufferHandle], _children: &dyn vortex_array::serde::ArrayChildren, session: &vortex_session::VortexSession) -> vortex_error::VortexResult> -pub fn vortex_array::arrays::Constant::execute(array: vortex_array::Array, _ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult +pub fn vortex_array::arrays::Constant::execute(array: vortex_array::Array, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult pub fn vortex_array::arrays::Constant::execute_parent(array: vortex_array::ArrayView<'_, Self>, parent: &vortex_array::ArrayRef, child_idx: usize, ctx: &mut vortex_array::ExecutionCtx) -> vortex_error::VortexResult> diff --git a/vortex-array/src/arrays/constant/vtable/canonical.rs b/vortex-array/src/arrays/constant/vtable/canonical.rs index 8d4955c2261..7d004ceb765 100644 --- a/vortex-array/src/arrays/constant/vtable/canonical.rs +++ b/vortex-array/src/arrays/constant/vtable/canonical.rs @@ -10,6 +10,7 @@ use vortex_error::VortexExpect; use vortex_error::VortexResult; use crate::Canonical; +use crate::ExecutionCtx; use crate::IntoArray; use crate::array::ArrayView; use crate::arrays::BoolArray; @@ -36,7 +37,10 @@ use crate::scalar::Scalar; use crate::validity::Validity; /// Shared implementation for both `canonicalize` and `execute` methods. -pub(crate) fn constant_canonicalize(array: ArrayView<'_, Constant>) -> VortexResult { +pub(crate) fn constant_canonicalize( + array: ArrayView<'_, Constant>, + ctx: &mut ExecutionCtx, +) -> VortexResult { let scalar = array.scalar(); let validity = match array.dtype().nullability() { @@ -163,7 +167,16 @@ pub(crate) fn constant_canonicalize(array: ArrayView<'_, Constant>) -> VortexRes let s = scalar.as_extension(); let storage_scalar = s.to_storage_scalar(); - let storage_self = ConstantArray::new(storage_scalar, array.len()).into_array(); + + // NB: We need to execute the constant array to be canonical because there is a + // reduction rule that turns `Extension(Constant(..))` into `Constant(Extension(..))`, + // and if we don't do this we create an infinite cycle. + // See `ExtensionConstantRule` for more details. + let storage_self = ConstantArray::new(storage_scalar, array.len()) + .into_array() + .execute::(ctx)? + .into_array(); + Canonical::Extension(ExtensionArray::new(ext_dtype.clone(), storage_self)) } DType::Variant(_) => { diff --git a/vortex-array/src/arrays/constant/vtable/mod.rs b/vortex-array/src/arrays/constant/vtable/mod.rs index 9b4f9dbb953..7384505d854 100644 --- a/vortex-array/src/arrays/constant/vtable/mod.rs +++ b/vortex-array/src/arrays/constant/vtable/mod.rs @@ -160,9 +160,10 @@ impl VTable for Constant { PARENT_RULES.evaluate(array, parent, child_idx) } - fn execute(array: Array, _ctx: &mut ExecutionCtx) -> VortexResult { + fn execute(array: Array, ctx: &mut ExecutionCtx) -> VortexResult { Ok(ExecutionResult::done(constant_canonicalize( array.as_view(), + ctx, )?)) } @@ -268,10 +269,11 @@ fn append_value_or_nulls( #[cfg(test)] mod tests { use rstest::rstest; + use vortex_error::VortexResult; use vortex_session::VortexSession; - use crate::ExecutionCtx; use crate::IntoArray; + use crate::VortexSessionExecute; use crate::arrays::ConstantArray; use crate::arrays::constant::vtable::canonical::constant_canonicalize; use crate::assert_arrays_eq; @@ -282,34 +284,29 @@ mod tests { use crate::dtype::StructFields; use crate::scalar::Scalar; - fn ctx() -> ExecutionCtx { - ExecutionCtx::new(VortexSession::empty()) - } - /// Appends `array` into a fresh builder and asserts the result matches `constant_canonicalize`. - fn assert_append_matches_canonical(array: ConstantArray) -> vortex_error::VortexResult<()> { - let expected = constant_canonicalize(array.as_view())?.into_array(); + fn assert_append_matches_canonical(array: ConstantArray) -> VortexResult<()> { + let mut ctx = VortexSession::empty().create_execution_ctx(); + + let expected = constant_canonicalize(array.as_view(), &mut ctx)?.into_array(); let mut builder = builder_with_capacity(array.dtype(), array.len()); array .into_array() - .append_to_builder(builder.as_mut(), &mut ctx())?; + .append_to_builder(builder.as_mut(), &mut ctx)?; let result = builder.finish(); assert_arrays_eq!(&result, &expected); Ok(()) } #[test] - fn test_null_constant_append() -> vortex_error::VortexResult<()> { + fn test_null_constant_append() -> VortexResult<()> { assert_append_matches_canonical(ConstantArray::new(Scalar::null(DType::Null), 5)) } #[rstest] #[case::bool_true(true, 5)] #[case::bool_false(false, 3)] - fn test_bool_constant_append( - #[case] value: bool, - #[case] n: usize, - ) -> vortex_error::VortexResult<()> { + fn test_bool_constant_append(#[case] value: bool, #[case] n: usize) -> VortexResult<()> { assert_append_matches_canonical(ConstantArray::new( Scalar::bool(value, Nullability::NonNullable), n, @@ -317,7 +314,7 @@ mod tests { } #[test] - fn test_bool_null_constant_append() -> vortex_error::VortexResult<()> { + fn test_bool_null_constant_append() -> VortexResult<()> { assert_append_matches_canonical(ConstantArray::new( Scalar::null(DType::Bool(Nullability::Nullable)), 4, @@ -332,7 +329,7 @@ mod tests { fn test_primitive_constant_append( #[case] scalar: Scalar, #[case] n: usize, - ) -> vortex_error::VortexResult<()> { + ) -> VortexResult<()> { assert_append_matches_canonical(ConstantArray::new(scalar, n)) } @@ -341,10 +338,7 @@ mod tests { #[case::utf8_noninline("hello world!!", 5)] // >12 bytes: requires buffer block #[case::utf8_empty("", 3)] #[case::utf8_n_zero("hello world!!", 0)] // n=0 with non-inline: must not write orphaned bytes - fn test_utf8_constant_append( - #[case] value: &str, - #[case] n: usize, - ) -> vortex_error::VortexResult<()> { + fn test_utf8_constant_append(#[case] value: &str, #[case] n: usize) -> VortexResult<()> { assert_append_matches_canonical(ConstantArray::new( Scalar::utf8(value, Nullability::NonNullable), n, @@ -352,7 +346,7 @@ mod tests { } #[test] - fn test_utf8_null_constant_append() -> vortex_error::VortexResult<()> { + fn test_utf8_null_constant_append() -> VortexResult<()> { assert_append_matches_canonical(ConstantArray::new( Scalar::null(DType::Utf8(Nullability::Nullable)), 4, @@ -362,10 +356,7 @@ mod tests { #[rstest] #[case::binary_inline(vec![1u8, 2, 3], 5)] // ≤12 bytes: inlined #[case::binary_noninline(vec![0u8; 13], 5)] // >12 bytes: buffer block - fn test_binary_constant_append( - #[case] value: Vec, - #[case] n: usize, - ) -> vortex_error::VortexResult<()> { + fn test_binary_constant_append(#[case] value: Vec, #[case] n: usize) -> VortexResult<()> { assert_append_matches_canonical(ConstantArray::new( Scalar::binary(value, Nullability::NonNullable), n, @@ -373,7 +364,7 @@ mod tests { } #[test] - fn test_binary_null_constant_append() -> vortex_error::VortexResult<()> { + fn test_binary_null_constant_append() -> VortexResult<()> { assert_append_matches_canonical(ConstantArray::new( Scalar::null(DType::Binary(Nullability::Nullable)), 4, @@ -381,7 +372,7 @@ mod tests { } #[test] - fn test_struct_constant_append() -> vortex_error::VortexResult<()> { + fn test_struct_constant_append() -> VortexResult<()> { let fields = StructFields::new( ["x", "y"].into(), vec![ @@ -400,7 +391,7 @@ mod tests { } #[test] - fn test_null_struct_constant_append() -> vortex_error::VortexResult<()> { + fn test_null_struct_constant_append() -> VortexResult<()> { let fields = StructFields::new( ["x"].into(), vec![DType::Primitive(PType::I32, Nullability::Nullable)], diff --git a/vortex-array/src/arrays/extension/vtable/canonical.rs b/vortex-array/src/arrays/extension/vtable/canonical.rs deleted file mode 100644 index 0d735177e5d..00000000000 --- a/vortex-array/src/arrays/extension/vtable/canonical.rs +++ /dev/null @@ -1,2 +0,0 @@ -// SPDX-License-Identifier: Apache-2.0 -// SPDX-FileCopyrightText: Copyright the Vortex contributors diff --git a/vortex-array/src/arrays/extension/vtable/mod.rs b/vortex-array/src/arrays/extension/vtable/mod.rs index a3fde75f79b..82ad85e701f 100644 --- a/vortex-array/src/arrays/extension/vtable/mod.rs +++ b/vortex-array/src/arrays/extension/vtable/mod.rs @@ -1,9 +1,5 @@ // SPDX-License-Identifier: Apache-2.0 // SPDX-FileCopyrightText: Copyright the Vortex contributors -mod canonical; -mod kernel; -mod operations; -mod validity; use std::hash::Hasher; @@ -36,6 +32,10 @@ use crate::buffer::BufferHandle; use crate::dtype::DType; use crate::serde::ArrayChildren; +mod kernel; +mod operations; +mod validity; + #[derive(Clone, Debug)] pub struct Extension;