Only support direct JSON inputs for expressions

Signed-off-by: Adam Gutglick <adam@spiraldb.com>

Author: AdamGS

encodings/parquet-variant/src/json_to_variant_tests.rs

@@ -4,8 +4,8 @@
 //! Execution tests for the `vortex.json_to_variant` scalar function.
 //!
 //! The function definition lives in `vortex-json`; the JSON->Variant construction is performed
-//! by the [`JsonToVariantKernel`](crate::kernel) registered here, so these end-to-end tests live
-//! in `vortex-parquet-variant` where that kernel is registered.
+//! by the execute-parent kernel registered here, so these end-to-end tests live in
+//! `vortex-parquet-variant` where that kernel is registered.
 
 use std::sync::LazyLock;
 
@@ -54,6 +54,10 @@ fn shred_field_as_i64(field: &str) -> VortexResult<ShreddingSpec> {
     ShreddingSpec::try_new([(VariantPath::field(field), i64_dtype())])
 }
 
+fn json_input(storage: ArrayRef) -> VortexResult<ArrayRef> {
+    Ok(ExtensionArray::try_new_from_vtable(Json, EmptyMetadata, storage)?.into_array())
+}
+
 fn execute_json_to_variant(input: ArrayRef, shredding: ShreddingSpec) -> VortexResult<ArrayRef> {
     let expr = json_to_variant(root(), shredding);
     input
@@ -82,8 +86,21 @@ fn assert_variant_i64_rows(array: &ArrayRef, expected: &[Option<i64>]) -> Vortex
 }
 
 #[test]
-fn converts_utf8_json_rows() -> VortexResult<()> {
-    let input = VarBinViewArray::from_iter_str([r#"{"a": 1}"#, "2", r#"{"a": 3}"#]).into_array();
+fn rejects_bare_utf8_input() {
+    let input = VarBinViewArray::from_iter_str(["1", "2"]).into_array();
+
+    let err = execute_json_to_variant(input, ShreddingSpec::empty()).unwrap_err();
+    assert!(
+        err.to_string().contains("Json extension"),
+        "unexpected error: {err}"
+    );
+}
+
+#[test]
+fn converts_json_extension_rows() -> VortexResult<()> {
+    let input = json_input(
+        VarBinViewArray::from_iter_str([r#"{"a": 1}"#, "2", r#"{"a": 3}"#]).into_array(),
+    )?;
 
     let result = execute_json_to_variant(input, ShreddingSpec::empty())?;
 
@@ -117,7 +134,7 @@ fn converts_utf8_json_rows() -> VortexResult<()> {
 #[test]
 fn converts_json_extension_input() -> VortexResult<()> {
     let storage = VarBinViewArray::from_iter_str(["1", "2"]).into_array();
-    let input = ExtensionArray::try_new_from_vtable(Json, EmptyMetadata, storage)?.into_array();
+    let input = json_input(storage)?;
 
     let result = execute_json_to_variant(input, ShreddingSpec::empty())?;
 
@@ -127,10 +144,10 @@ fn converts_json_extension_input() -> VortexResult<()> {
 
 #[test]
 fn dict_encoded_input_converts_each_row() -> VortexResult<()> {
-    // A dictionary-encoded string column exercises the dict-pushdown / canonicalization path:
+    // A dictionary-encoded JSON column exercises the dict-pushdown / canonicalization path:
     // `json_to_variant` is not null-sensitive, so it pushes into the dict values (canonical
-    // strings) where the kernel fires; either way every row must convert correctly.
-    let values = VarBinViewArray::from_iter_str(["1", "2"]).into_array();
+    // JSON extension values) where the kernel fires; either way every row must convert correctly.
+    let values = json_input(VarBinViewArray::from_iter_str(["1", "2"]).into_array())?;
     let codes = PrimitiveArray::from_iter([0u8, 1, 0, 1, 0]).into_array();
     let input = DictArray::try_new(codes, values)?.into_array();
 
@@ -142,8 +159,9 @@ fn dict_encoded_input_converts_each_row() -> VortexResult<()> {
 
 #[test]
 fn null_rows_stay_null_and_json_null_becomes_variant_null() -> VortexResult<()> {
-    let input =
-        VarBinViewArray::from_iter_nullable_str([Some("1"), None, Some("null")]).into_array();
+    let input = json_input(
+        VarBinViewArray::from_iter_nullable_str([Some("1"), None, Some("null")]).into_array(),
+    )?;
 
     let result = execute_json_to_variant(input, ShreddingSpec::empty())?;
 
@@ -158,22 +176,26 @@ fn null_rows_stay_null_and_json_null_becomes_variant_null() -> VortexResult<()>
 }
 
 #[test]
-fn invalid_json_errors() {
-    let input = VarBinViewArray::from_iter_str([r#"{"a": 1}"#, r#"{"a":"#]).into_array();
+fn invalid_json_errors() -> VortexResult<()> {
+    let input =
+        json_input(VarBinViewArray::from_iter_str([r#"{"a": 1}"#, r#"{"a":"#]).into_array())?;
 
     let err = execute_json_to_variant(input, ShreddingSpec::empty()).unwrap_err();
     assert!(!err.to_string().is_empty());
+    Ok(())
 }
 
 #[test]
 fn shredding_produces_typed_value_child() -> VortexResult<()> {
-    let input = VarBinViewArray::from_iter_str([
-        r#"{"a": 1, "b": "x"}"#,
-        r#"{"a": 2, "b": "y"}"#,
-        r#"{"a": "not-a-number", "b": "z"}"#,
-        r#"{"b": "missing-a"}"#,
-    ])
-    .into_array();
+    let input = json_input(
+        VarBinViewArray::from_iter_str([
+            r#"{"a": 1, "b": "x"}"#,
+            r#"{"a": 2, "b": "y"}"#,
+            r#"{"a": "not-a-number", "b": "z"}"#,
+            r#"{"b": "missing-a"}"#,
+        ])
+        .into_array(),
+    )?;
 
     let result = execute_json_to_variant(input, shred_field_as_i64("a")?)?;
 
@@ -227,9 +249,10 @@ fn shredding_produces_typed_value_child() -> VortexResult<()> {
 
 #[test]
 fn shredding_preserves_null_rows() -> VortexResult<()> {
-    let input =
+    let input = json_input(
         VarBinViewArray::from_iter_nullable_str([Some(r#"{"a": 1}"#), None, Some(r#"{"a": 3}"#)])
-            .into_array();
+            .into_array(),
+    )?;
 
     let result = execute_json_to_variant(input, shred_field_as_i64("a")?)?;
 
@@ -253,7 +276,8 @@ fn shredding_preserves_null_rows() -> VortexResult<()> {
 
 #[test]
 fn shredding_root_path_shreds_top_level_values() -> VortexResult<()> {
-    let input = VarBinViewArray::from_iter_str(["1", "2", r#""not-a-number""#]).into_array();
+    let input =
+        json_input(VarBinViewArray::from_iter_str(["1", "2", r#""not-a-number""#]).into_array())?;
     let spec = ShreddingSpec::try_new([(VariantPath::root(), i64_dtype())])?;
 
     let result = execute_json_to_variant(input, spec)?;

encodings/parquet-variant/src/kernel.rs

@@ -24,7 +24,6 @@ use vortex_array::arrays::Dict;
 use vortex_array::arrays::Extension;
 use vortex_array::arrays::Filter;
 use vortex_array::arrays::Slice;
-use vortex_array::arrays::VarBinView;
 use vortex_array::arrays::dict::TakeExecute;
 use vortex_array::arrays::dict::TakeExecuteAdaptor;
 use vortex_array::arrays::extension::ExtensionArrayExt;
@@ -72,7 +71,6 @@ pub(crate) fn initialize(session: &VortexSession) {
         TakeExecuteAdaptor(ParquetVariant),
     );
     kernels.register_execute_parent_kernel(VariantGet.id(), ParquetVariant, VariantGetKernel);
-    kernels.register_execute_parent_kernel(JsonToVariant.id(), VarBinView, JsonToVariantKernel);
     kernels.register_execute_parent_kernel(
         JsonToVariant.id(),
         Extension,
@@ -120,13 +118,13 @@ impl ExecuteParentKernel<ParquetVariant> for VariantGetKernel {
     }
 }
 
-/// Performs the [`JsonToVariant`] conversion (and optional shredding) over a JSON string array.
+/// Performs the [`JsonToVariant`] conversion (and optional shredding) over JSON string storage.
 ///
 /// `JsonToVariant`'s definition lives in `vortex-json`; the registered `execute_parent` kernels
 /// delegate here to do the actual JSON parsing and optional shredding using
-/// `parquet_variant_compute`, producing a [`ParquetVariant`] array. `strings` is the input JSON
-/// string array (any string encoding); it is executed to Arrow and parsed. Nullability of the
-/// result follows `parent.dtype()`, which equals the input's nullability.
+/// `parquet_variant_compute`, producing a [`ParquetVariant`] array. `strings` is the JSON
+/// extension's storage array; it is executed to Arrow and parsed. Nullability of the result follows
+/// `parent.dtype()`, which equals the input's nullability.
 fn json_strings_to_variant(
     strings: ArrayRef,
     parent: ScalarFnArrayView<'_, JsonToVariant>,
@@ -156,32 +154,10 @@ fn json_strings_to_variant(
     }
 }
 
-/// Builds Parquet Variant arrays for [`JsonToVariant`] over canonical `VarBinView` string input.
-#[derive(Default, Debug)]
-struct JsonToVariantKernel;
-
-impl ExecuteParentKernel<VarBinView> for JsonToVariantKernel {
-    type Parent = ExactScalarFn<JsonToVariant>;
-
-    fn execute_parent(
-        &self,
-        array: ArrayView<'_, VarBinView>,
-        parent: ScalarFnArrayView<'_, JsonToVariant>,
-        child_idx: usize,
-        ctx: &mut ExecutionCtx,
-    ) -> VortexResult<Option<ArrayRef>> {
-        if child_idx != 0 {
-            return Ok(None);
-        }
-        json_strings_to_variant(array.as_ref().clone(), parent, ctx).map(Some)
-    }
-}
-
 /// Builds Parquet Variant arrays for [`JsonToVariant`] over a [`Json`] extension input.
 ///
-/// `JsonToVariant` also accepts `Json` extension values directly. This kernel unwraps the
-/// extension's string storage and runs the same conversion as [`JsonToVariantKernel`]. It is keyed
-/// on the shared extension encoding, so it declines any non-`Json` extension.
+/// This kernel unwraps the extension's string storage and runs the JSON conversion. It is keyed on
+/// the shared extension encoding, so it declines any non-`Json` extension.
 #[derive(Default, Debug)]
 struct JsonExtensionToVariantKernel;
 

vortex-json/src/json_to_variant.rs

@@ -13,8 +13,6 @@ use vortex_array::ExecutionCtx;
 use vortex_array::IntoArray;
 use vortex_array::arrays::Extension;
 use vortex_array::arrays::ExtensionArray;
-use vortex_array::arrays::VarBinView;
-use vortex_array::arrays::VarBinViewArray;
 use vortex_array::arrays::scalar_fn::ScalarFnFactoryExt;
 use vortex_array::dtype::DType;
 use vortex_array::expr::Expression;
@@ -38,9 +36,9 @@ use crate::Json;
 
 /// Parses JSON strings into Variant values, optionally shredding fields.
 ///
-/// Accepts `Utf8` inputs or [`Json`] extension inputs and returns `Variant` values with the
-/// input's nullability. Null rows stay null, the JSON literal `null` becomes a variant-null
-/// value, and any row that fails to parse as JSON fails the whole expression.
+/// Accepts [`Json`] extension inputs and returns `Variant` values with the input's nullability.
+/// Null rows stay null, the JSON literal `null` becomes a variant-null value, and any row that
+/// fails to parse as JSON fails the whole expression.
 ///
 /// A non-empty [`ShreddingSpec`] additionally shreds the selected paths into a typed shredded
 /// child, following the [Parquet Variant shredding] rules: rows whose value does not match the
@@ -50,10 +48,10 @@ use crate::Json;
 ///
 /// Building a Variant requires a concrete Variant encoding, so this function does not perform the
 /// conversion itself. The Variant encoding registered with the session supplies it as an
-/// `execute_parent` kernel keyed on the canonical string encodings (`VarBinView` for `Utf8`, and
-/// the extension encoding for a [`Json`] input). The fallback [`execute`](ScalarFnVTable::execute)
-/// here only canonicalizes its input to one of those encodings and re-dispatches so that kernel
-/// runs; it errors if no Variant encoding is registered with the session.
+/// `execute_parent` kernel keyed on the extension encoding for a [`Json`] input. The fallback
+/// [`execute`](ScalarFnVTable::execute) here only canonicalizes the input to that encoding and
+/// re-dispatches so that kernel runs; it errors if no Variant encoding is registered with the
+/// session.
 ///
 /// # Normalization
 ///
@@ -135,11 +133,10 @@ impl ScalarFnVTable for JsonToVariant {
     fn return_dtype(&self, _options: &Self::Options, arg_dtypes: &[DType]) -> VortexResult<DType> {
         let input_dtype = &arg_dtypes[0];
         vortex_ensure!(
-            input_dtype.is_utf8()
-                || input_dtype
-                    .as_extension_opt()
-                    .is_some_and(|ext_dtype| ext_dtype.is::<Json>()),
-            "JsonToVariant input must be Utf8 or a Json extension, found {input_dtype}"
+            input_dtype
+                .as_extension_opt()
+                .is_some_and(|ext_dtype| ext_dtype.is::<Json>()),
+            "JsonToVariant input must be a Json extension, found {input_dtype}"
         );
 
         Ok(DType::Variant(input_dtype.nullability()))
@@ -154,24 +151,19 @@ impl ScalarFnVTable for JsonToVariant {
         let input = args.get(0)?;
 
         // This function does not build Variants itself: the Variant encoding registered with the
-        // session supplies the conversion as an `execute_parent` kernel keyed on the canonical
-        // string encodings (`VarBinView` for `Utf8`, the extension encoding for a `Json` input).
-        // Reaching this fallback means no such kernel fired, so canonicalize the input to one of
-        // those encodings and re-dispatch. If the input is already canonical here, no kernel is
-        // registered for it; bail with a clear error rather than looping.
+        // session supplies the conversion as an `execute_parent` kernel keyed on the extension
+        // encoding for a `Json` input. Reaching this fallback means no such kernel fired, so
+        // canonicalize the input to that encoding and re-dispatch. If the input is already
+        // canonical here, no kernel is registered for it; bail with a clear error rather than
+        // looping.
         let no_kernel = || {
             vortex_err!(
                 "json_to_variant requires a registered Variant encoding to build Variant values \
                  from JSON, but none is registered with this session"
             )
         };
 
-        let canonical = if input.dtype().is_utf8() {
-            if input.is::<VarBinView>() {
-                return Err(no_kernel());
-            }
-            input.execute::<VarBinViewArray>(ctx)?.into_array()
-        } else if input
+        let canonical = if input
             .dtype()
             .as_extension_opt()
             .is_some_and(|ext_dtype| ext_dtype.is::<Json>())
@@ -182,7 +174,7 @@ impl ScalarFnVTable for JsonToVariant {
             input.execute::<ExtensionArray>(ctx)?.into_array()
         } else {
             vortex_bail!(
-                "JsonToVariant input must be Utf8 or a Json extension, found {}",
+                "JsonToVariant input must be a Json extension, found {}",
                 input.dtype()
             );
         };
@@ -195,16 +187,16 @@ impl ScalarFnVTable for JsonToVariant {
         // `json_to_variant` maps null rows to null rows and the JSON literal `null` to a
         // variant-null value independently of which other rows are null, so it commutes with
         // validity masking. Marking it not null-sensitive also lets it push through dictionaries
-        // into their (canonical-string) values, where the kernel fires directly.
+        // into their JSON extension values, where the kernel fires directly.
         false
     }
 }
 
 /// Creates a [`JsonToVariant`] expression that parses `child`'s JSON strings into Variant
 /// values, shredding the paths selected by `shredding`.
 ///
-/// `child` must produce `Utf8` or [`Json`] extension values; the result is `Variant` with the
-/// input's nullability. Rows containing invalid JSON fail the expression.
+/// `child` must produce [`Json`] extension values; the result is `Variant` with the input's
+/// nullability. Rows containing invalid JSON fail the expression.
 ///
 /// Note that this is a lossy, normalizing conversion. See [`JsonToVariant`] for the full list of
 /// caveats.
@@ -360,36 +352,45 @@ mod tests {
         Ok(())
     }
 
+    #[test]
+    fn utf8_input_is_rejected() {
+        let input = VarBinViewArray::from_iter_str(["1", "2"]).into_array();
+        let err = JsonToVariant
+            .try_new_array(input.len(), JsonToVariantOptions::unshredded(), [input])
+            .unwrap_err();
+
+        assert!(
+            err.to_string().contains("Json extension"),
+            "unexpected error: {err}"
+        );
+    }
+
     #[test]
     fn execute_without_variant_kernel_errors() -> VortexResult<()> {
         // With no Variant encoding registered, executing over an already-canonical input must
-        // surface a clear error rather than looping. Both canonical input forms are covered: a
-        // bare `Utf8`/`VarBinView`, and a `Json` extension.
-        let utf8 = VarBinViewArray::from_iter_str(["1", "2"]).into_array();
-        let json_extension = ExtensionArray::try_new_from_vtable(
+        // surface a clear error rather than looping.
+        let input = ExtensionArray::try_new_from_vtable(
             Json,
             EmptyMetadata,
             VarBinViewArray::from_iter_str(["1", "2"]).into_array(),
         )?
         .into_array();
 
-        for input in [utf8, json_extension] {
-            let dtype = input.dtype().clone();
-            let array = JsonToVariant.try_new_array(
-                input.len(),
-                JsonToVariantOptions::unshredded(),
-                [input],
-            )?;
-
-            let err = array
-                .execute::<ArrayRef>(&mut session().create_execution_ctx())
-                .unwrap_err();
-
-            assert!(
-                err.to_string().contains("Variant encoding"),
-                "unexpected error for {dtype}: {err}"
-            );
-        }
+        let dtype = input.dtype().clone();
+        let array = JsonToVariant.try_new_array(
+            input.len(),
+            JsonToVariantOptions::unshredded(),
+            [input],
+        )?;
+
+        let err = array
+            .execute::<ArrayRef>(&mut session().create_execution_ctx())
+            .unwrap_err();
+
+        assert!(
+            err.to_string().contains("Variant encoding"),
+            "unexpected error for {dtype}: {err}"
+        );
         Ok(())
     }
 }