adriangb commented on code in PR #23130:
URL: https://github.com/apache/datafusion/pull/23130#discussion_r3552021176
##########
datafusion/physical-expr/src/scalar_subquery.rs:
##########
@@ -59,19 +59,6 @@ impl ScalarSubqueryExpr {
}
}
- pub fn data_type(&self) -> &DataType {
- &self.data_type
- }
-
- pub fn nullable(&self) -> bool {
- self.nullable
- }
-
- /// Returns the index of this subquery in the shared results container.
- pub fn index(&self) -> SubqueryIndex {
- self.index
- }
Review Comment:
We should deprecate this according to
https://datafusion.apache.org/contributor-guide/api-health.html and open a
tracking issue to remove them.
##########
datafusion/physical-expr/src/scalar_subquery.rs:
##########
@@ -139,6 +126,64 @@ impl PhysicalExpr for ScalarSubqueryExpr {
fn fmt_sql(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "(scalar subquery)")
}
+
+ #[cfg(feature = "proto")]
+ fn try_to_proto(
+ &self,
+ _ctx:
&datafusion_physical_expr_common::physical_expr::proto_encode::PhysicalExprEncodeCtx<'_>,
+ ) -> Result<Option<datafusion_proto_models::protobuf::PhysicalExprNode>> {
+ use datafusion_proto_models::protobuf;
+ Ok(Some(protobuf::PhysicalExprNode {
+ expr_id: None,
+ expr_type:
Some(protobuf::physical_expr_node::ExprType::ScalarSubquery(
+ protobuf::PhysicalScalarSubqueryExprNode {
+ data_type: Some((&self.data_type).try_into()?),
+ nullable: self.nullable,
+ index: self.index.as_usize() as u32,
Review Comment:
This is just carried over, but it looks like it could be a bug. Could we
address it here?
```suggestion
index: u32::try_from(self.index.as_usize()).map_err(|_| {
internal_datafusion_err!(
"scalar subquery index {} does not fit in u32",
self.index.as_usize()
)
})?,
```
--
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]
