kosiew commented on code in PR #22313:
URL: https://github.com/apache/datafusion/pull/22313#discussion_r3265145973
##########
datafusion/optimizer/src/scalar_subquery_to_join.rs:
##########
@@ -156,14 +156,12 @@ impl OptimizerRule for ScalarSubqueryToJoin {
#[allow(clippy::allow_attributes, clippy::mutable_key_type)]
// Expr contains Arc with interior mutability but is
intentionally used as hash key
let mut expr_to_rewrite_expr_map = HashMap::new();
- #[allow(clippy::allow_attributes, clippy::mutable_key_type)]
- // Expr contains Arc with interior mutability but is
intentionally used as hash key
- let mut subquery_to_expr_map = HashMap::new();
+ let mut alias_to_expr_map: HashMap<String, Expr> =
HashMap::new();
Review Comment:
Nice catch on the alias mapping issue. One thought: it might be a little
simpler and more robust to key the projection rewrite bookkeeping by projection
index, or store alias -> projection index, instead of alias -> cloned Expr plus
Expr -> rewrite Expr.
The current fix works because the generated aliases are unique, but using
indexes would encode the ownership relationship directly and avoid another
structural Expr lookup in this already pretty subtle rewrite path.
--
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]
