Phoenix500526 commented on code in PR #23915:
URL: https://github.com/apache/datafusion/pull/23915#discussion_r3683474700
##########
datafusion/physical-plan/src/scalar_subquery.rs:
##########
@@ -254,6 +254,68 @@ impl ExecutionPlan for ScalarSubqueryExec {
fn cardinality_effect(&self) -> CardinalityEffect {
CardinalityEffect::Equal
}
+
+ #[cfg(feature = "proto")]
+ fn try_to_proto(
+ &self,
+ ctx: &crate::proto::ExecutionPlanEncodeCtx<'_>,
+ ) -> Result<Option<datafusion_proto_models::protobuf::PhysicalPlanNode>> {
+ use datafusion_proto_models::protobuf;
+
+ let input = ctx.encode_child(self.input())?;
+ // Subquery indices are positional and recovered during decoding.
+ let subqueries =
+ ctx.encode_children(self.subqueries().iter().map(|subquery|
&subquery.plan))?;
+ Ok(Some(protobuf::PhysicalPlanNode {
+ physical_plan_type: Some(
+
protobuf::physical_plan_node::PhysicalPlanType::ScalarSubquery(Box::new(
+ protobuf::ScalarSubqueryExecNode {
+ input: Some(Box::new(input)),
+ subqueries,
+ },
+ )),
+ ),
+ }))
+ }
+}
+
+#[cfg(feature = "proto")]
+impl ScalarSubqueryExec {
+ /// Reconstruct a [`ScalarSubqueryExec`] from its protobuf representation.
+ pub fn try_from_proto(
+ node: &datafusion_proto_models::protobuf::PhysicalPlanNode,
+ ctx: &crate::proto::ExecutionPlanDecodeCtx<'_>,
+ ) -> Result<Arc<dyn ExecutionPlan>> {
+ use datafusion_proto_models::protobuf;
+
+ let scalar_subquery = crate::expect_plan_variant!(
+ node,
+ protobuf::physical_plan_node::PhysicalPlanType::ScalarSubquery,
+ "ScalarSubqueryExec",
+ );
+ let results =
ScalarSubqueryResults::new(scalar_subquery.subqueries.len());
+ let input_node = scalar_subquery.input.as_deref().ok_or_else(|| {
+ datafusion_common::internal_datafusion_err!(
+ "ScalarSubqueryExec is missing required field 'input'"
+ )
+ })?;
Review Comment:
`decode_required_child` returns an already decoded execution plan, but this
input must be decoded through
`decode_child_with_scalar_subquery_results` so that its `ScalarSubqueryExpr`
nodes receive the shared results container.
`ScalarSubqueryExec` is the only owner of this scoped decoding behavior, so
adding a required variant would introduce a single-use API. I think keeping the
explicit required-field check here is simpler.
--
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]