LinSimon-901101 commented on code in PR #5889:
URL: https://github.com/apache/datafusion-comet/pull/5889#discussion_r4003214524


##########
native/core/src/execution/expressions/subquery.rs:
##########
@@ -51,6 +58,81 @@ impl Subquery {
             exec_context_id,
             id,
             data_type,
+            struct_value: OnceLock::new(),
+        }
+    }
+}
+
+impl PartialEq for Subquery {
+    fn eq(&self, other: &Self) -> bool {
+        self.exec_context_id == other.exec_context_id
+            && self.id == other.id
+            && self.data_type == other.data_type
+    }
+}
+
+impl Eq for Subquery {}
+
+impl Hash for Subquery {
+    fn hash<H: Hasher>(&self, state: &mut H) {
+        self.exec_context_id.hash(state);
+        self.id.hash(state);
+        self.data_type.hash(state);
+    }
+}
+
+/// The JVM bridge emits one row with one struct column. Validate the wire 
shape and type before
+/// creating the scalar; Arrow IPC validation also keeps malformed strings out 
of native arrays.
+fn decode_struct_result(
+    bytes: &[u8],
+    data_type: &DataType,
+) -> datafusion::common::Result<ScalarValue> {
+    let mut reader = StreamReader::try_new(Cursor::new(bytes), None)?;
+    let Some(batch) = reader.next().transpose()? else {
+        return internal_err!("Scalar subquery IPC result contains no batch");
+    };
+    if batch.num_rows() != 1 || batch.num_columns() != 1 {
+        return internal_err!("Scalar subquery IPC result must contain one row 
and one column");
+    }
+    if reader.next().transpose()?.is_some() {
+        return internal_err!("Scalar subquery IPC result contains more than 
one batch");
+    }
+    let value = align_struct_metadata(batch.column(0), data_type)?;

Review Comment:
   Thanks for pointing this out. I've added a JVM serializer test to prove that 
the Arrow IPC wire schema loses nested Parquet field IDs, while the planned 
protobuf type retains metadata. A native integration test also verifies that 
the output after alignment restores nested IDs and covers NULL children and 
NULL structs.
   
   Tests: JVM serializer coverage in 
[CometArrowStreamSuite.scala:65–140](https://github.com/apache/datafusion-comet/blob/fb658a6918149d5b328c2cf5dea10c002b9d75ab/spark/src/test/scala/org/apache/spark/sql/comet/execution/arrow/CometArrowStreamSuite.scala#L65-L140),
 and native-output metadata/NULL coverage in 
[CometExecSuite.scala:2375–2480](https://github.com/apache/datafusion-comet/blob/fb658a6918149d5b328c2cf5dea10c002b9d75ab/spark/src/test/scala/org/apache/comet/exec/CometExecSuite.scala#L2375-L2480).



##########
native/core/src/execution/expressions/subquery.rs:
##########
@@ -51,6 +58,81 @@ impl Subquery {
             exec_context_id,
             id,
             data_type,
+            struct_value: OnceLock::new(),
+        }
+    }
+}
+
+impl PartialEq for Subquery {
+    fn eq(&self, other: &Self) -> bool {
+        self.exec_context_id == other.exec_context_id
+            && self.id == other.id
+            && self.data_type == other.data_type
+    }
+}
+
+impl Eq for Subquery {}
+
+impl Hash for Subquery {
+    fn hash<H: Hasher>(&self, state: &mut H) {
+        self.exec_context_id.hash(state);
+        self.id.hash(state);
+        self.data_type.hash(state);
+    }
+}
+
+/// The JVM bridge emits one row with one struct column. Validate the wire 
shape and type before
+/// creating the scalar; Arrow IPC validation also keeps malformed strings out 
of native arrays.
+fn decode_struct_result(

Review Comment:
   These shape checks cannot fail on the current supported production path, 
since the JVM serializer always emits one row, one column, and one batch. I've 
removed the extra-batch probe and retained the no-batch and row/column guards 
to prevent panics if an internal bug violates that contract.
   
   Code: the fixed producer shape in 
[CometArrowConverters.scala:62–76](https://github.com/apache/datafusion-comet/blob/fb658a6918149d5b328c2cf5dea10c002b9d75ab/spark/src/main/scala/org/apache/spark/sql/comet/execution/arrow/CometArrowConverters.scala#L62-L76),
 and the simplified decoder with retained defensive guards in 
[subquery.rs:89–105](https://github.com/apache/datafusion-comet/blob/fb658a6918149d5b328c2cf5dea10c002b9d75ab/native/core/src/execution/expressions/subquery.rs#L89-L105).



-- 
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]

Reply via email to