alamb commented on code in PR #26164:
URL: https://github.com/apache/datafusion/pull/26164#discussion_r4233121415
##########
datafusion/expr/src/expr_schema.rs:
##########
@@ -435,6 +435,24 @@ impl ExprSchemable for Expr {
Expr::ScalarSubquery(subquery) =>
Ok(scalar_subquery_nullable(subquery)),
Expr::BinaryExpr(BinaryExpr { left, right, op }) => match op {
Operator::IsDistinctFrom | Operator::IsNotDistinctFrom =>
Ok(false),
+ Operator::And => {
+ if contains_is_not_null(left.as_ref(), right.as_ref()) {
Review Comment:
this feels like treating the symptom rather than the root cause -- why would
we need to special case `IS NOT NULL` as part of `AND / OR` ? Wouldn't it make
more sense to perhaps set nullability for IsNotNull itself?
##########
datafusion/core/tests/issue_25978.rs:
##########
@@ -0,0 +1,27 @@
+use datafusion::prelude::*;
+use datafusion::error::Result;
+
+#[tokio::test]
+async fn test_issue_25978() -> Result<()> {
Review Comment:
please write this as a slt test
As it is, this test only verifies the query doesn't error -- using slt you
can aso verify its correctness
##########
datafusion/physical-optimizer/src/join_selection.rs:
##########
@@ -301,32 +301,40 @@ fn statistical_join_selection_subrule(
context: &dyn PhysicalOptimizerContext,
) -> Result<Transformed<Arc<dyn ExecutionPlan>>> {
let transformed = if let Some(hash_join) =
plan.downcast_ref::<HashJoinExec>() {
- match hash_join.partition_mode() {
- PartitionMode::Auto => try_collect_left(hash_join, false, context)?
- .map_or_else(
- || partitioned_hash_join(hash_join, context).map(Some),
- |v| Ok(Some(v)),
- )?,
- PartitionMode::CollectLeft => try_collect_left(hash_join, true,
context)?
- .map_or_else(
- || partitioned_hash_join(hash_join, context).map(Some),
- |v| Ok(Some(v)),
- )?,
- PartitionMode::Partitioned => {
- let left = hash_join.left();
- let right = hash_join.right();
- if can_swap_hash_join(hash_join)
- && should_swap_join_order(&**left, &**right, context)?
- {
- // Null-aware RightAnti only supports CollectLeft
- let partition_mode = if hash_join.null_aware {
- PartitionMode::CollectLeft
+ if !hash_join.dynamic_expressions_produced().is_empty() {
Review Comment:
is this tested anywhere? How is it related to the other fix?
--
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]
