rich7420 commented on code in PR #6589:
URL: https://github.com/apache/datafusion-comet/pull/6589#discussion_r4363008764
##########
native/core/src/execution/jni_api.rs:
##########
@@ -1665,20 +1665,19 @@ fn log_plan_metrics(exec_context: &ExecutionContext,
stage_id: jint, partition:
}
}
-fn convert_datatype_arrays(
+fn convert_field_arrays(
Review Comment:
Could we keep a `convert_datatype_arrays` wrapper? The new
`writeSortedPartitionsNative` on `main` calls it, so merging would break
compilation. `writeSortedFileNative` could use it too.
```suggestion
fn convert_datatype_arrays(
env: &mut Env,
serialized_datatypes: JObjectArray,
) -> JNIResult<Vec<ArrowDataType>> {
Ok(convert_field_arrays(env, serialized_datatypes)?
.into_iter()
.map(|field| field.data_type().clone())
.collect())
}
fn convert_field_arrays(
```
##########
spark/src/test/resources/sql-tests/expressions/misc/variant.sql:
##########
@@ -143,3 +145,43 @@ INSERT INTO test_plain_variant_shape VALUES
-- An ordinary binary struct with Variant-like field names is not a logical
Variant.
query
SELECT payload FROM test_plain_variant_shape ORDER BY id
+
+-- Arrow Variant storage must reach Spark UnsafeRow through native C2R.
+statement
+SET spark.sql.variant.allowReadingShredded=true
+
+statement
+SET spark.comet.exec.columnarToRow.native.enabled=true
+
+statement
+INSERT INTO test_variant VALUES
+ (5, parse_json('[1,"text",false]'), 'array'),
+ (6, parse_json('42'), 'scalar'),
+ (7, parse_json('{}'), 'empty-object'),
+ (8, parse_json('[]'), 'empty-array')
+
+-- Selecting only Variant or placing siblings on either side previously used
Spark C2R.
+query expect_operator(CometNativeColumnarToRowExec)
+SELECT v FROM test_variant
+
+query expect_operator(CometNativeColumnarToRowExec)
+SELECT id, v, tail FROM test_variant
+
+query expect_operator(CometNativeColumnarToRowExec)
+SELECT v, tail, id FROM test_variant
+
+statement
+SET spark.sql.variant.writeShredding.enabled=true
+
+statement
+CREATE TABLE test_variant_c2r_shredded USING parquet AS SELECT * FROM
test_variant
+
+query expect_operator(CometNativeColumnarToRowExec)
+SELECT id, v, tail FROM test_variant_c2r_shredded
+
+-- Disabling native conversion still produces the same rows through Spark C2R.
+statement
+SET spark.comet.exec.columnarToRow.native.enabled=false
+
+query expect_operator(ColumnarToRowExec)
Review Comment:
Could we also check that the scan stays native? Checking only
`ColumnarToRowExec` would still pass if the Comet scan fell back.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
