jayshrivastava commented on code in PR #22451:
URL: https://github.com/apache/datafusion/pull/22451#discussion_r3290458057
##########
datafusion/physical-plan/src/joins/hash_join/partitioned_hash_eval.rs:
##########
@@ -289,6 +289,38 @@ impl PartialEq for HashTableLookupExpr {
impl Eq for HashTableLookupExpr {}
+impl HashTableLookupExpr {
+ /// Serialize this expression to protobuf.
+ ///
+ /// `HashTableLookupExpr` holds an `Arc<Map>` (a runtime hash table built
+ /// on the build side) which cannot be serialized. We replace it with
+ /// `lit(true)`, which is safe because:
+ ///
+ /// - The filter is a performance optimisation, not a correctness
requirement.
+ /// - `lit(true)` passes all rows so no valid rows are lost.
+ /// - In distributed execution the remote worker has no access to the
+ /// build-side hash table anyway.
+ pub fn try_to_proto(
Review Comment:
I think this must be moved to the `impl PhysicalExpr for
HashTableLookupExpr` block to actually be used.
The way to test this code is being used is to delete the old paths in
`datafusion/datafusion/proto/src/physical_plan/to_proto.rs` and
`datafusion/datafusion/proto/src/physical_plan/from_proto.rs`.
Since this is a small PR, you may want to considering adding
`try_from_proto` as well :)
--
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]
