You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "davidhewitt (via GitHub)" <gi...@apache.org> on 2025/08/06 09:26:14 UTC

[PR] fix return field for `is_not_null` expression [datafusion]

davidhewitt opened a new pull request, #17056:
URL: https://github.com/apache/datafusion/pull/17056

   ## Which issue does this PR close?
   
   - Closes #17055.
   
   ## What changes are included in this PR?
   
   Removes incorrect `return_field` implementation from `IsNotNull` physical expression - it was forwarding to its input which meant it was incorrectly claiming the return datatype was the same as the input.
   
   ## Are these changes tested?
   
   Yes.
   
   ## Are there any user-facing changes?
   
   Bugfix.


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] fix return field for `is_not_null` expression [datafusion]

Posted by "findepi (via GitHub)" <gi...@apache.org>.
findepi commented on PR #17056:
URL: https://github.com/apache/datafusion/pull/17056#issuecomment-3163352022

   @davidhewitt can you please check CI output?


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] fix return field for `is_not_null` expression [datafusion]

Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on code in PR #17056:
URL: https://github.com/apache/datafusion/pull/17056#discussion_r2256990158


##########
datafusion/sqllogictest/test_files/expr.slt:
##########
@@ -2131,3 +2131,18 @@ query T
 select E'foo\t\tbar';
 ----
 foo		bar
+
+statement ok
+create table t (a float) as values (1), (2), (3);

Review Comment:
   ```suggestion
   create table t (a float) as values (1), (null), (3);
   ```
   



##########
datafusion/sqllogictest/test_files/expr.slt:
##########
@@ -2131,3 +2131,18 @@ query T
 select E'foo\t\tbar';
 ----
 foo		bar
+
+statement ok
+create table t (a float) as values (1), (2), (3);
+
+# https://github.com/apache/datafusion/issues/17055
+# is not null did not correctly infer as boolean in udf argument position
+query B
+select greatest(a is not null, false) from t;
+----
+true
+true

Review Comment:
   ```suggestion
   false
   ```
   



-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] fix return field for `is_not_null` expression [datafusion]

Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on PR #17056:
URL: https://github.com/apache/datafusion/pull/17056#issuecomment-3161168368

   > Looking again, it seems that `is_null` has the same problem. Should I update this PR accordingly?
   
   Yes lets do it in this PR!


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] fix return field for `is_not_null` expression [datafusion]

Posted by "davidhewitt (via GitHub)" <gi...@apache.org>.
davidhewitt commented on PR #17056:
URL: https://github.com/apache/datafusion/pull/17056#issuecomment-3161139227

   Thanks, accepted that revision 👍 


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] fix return field for `is_not_null` expression [datafusion]

Posted by "davidhewitt (via GitHub)" <gi...@apache.org>.
davidhewitt commented on PR #17056:
URL: https://github.com/apache/datafusion/pull/17056#issuecomment-3163040675

   Done 👍 


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] fix return field for `is_not_null` expression [datafusion]

Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on code in PR #17056:
URL: https://github.com/apache/datafusion/pull/17056#discussion_r2257951256


##########
datafusion/physical-expr/src/expressions/is_not_null.rs:
##########
@@ -94,10 +93,6 @@ impl PhysicalExpr for IsNotNullExpr {
         }
     }
 
-    fn return_field(&self, input_schema: &Schema) -> Result<FieldRef> {
-        self.arg.return_field(input_schema)
-    }

Review Comment:
   This makes sense, `is_null` always returns a non-nullable boolean, it does not depend on the input. The default implementation looks like:
   
   https://github.com/apache/datafusion/blob/73b50536570cbf87f95bbebd4952b8017dfe25b6/datafusion/physical-expr-common/src/physical_expr.rs#L86-L92
   
   Which is exactly what we want.



-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] fix return field for `is_not_null` expression [datafusion]

Posted by "davidhewitt (via GitHub)" <gi...@apache.org>.
davidhewitt commented on PR #17056:
URL: https://github.com/apache/datafusion/pull/17056#issuecomment-3163372501

   Sorry about that, editor autoformat stripped too much whitespace. Cleaned up the diff.


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] fix return field for `is_not_null` expression [datafusion]

Posted by "davidhewitt (via GitHub)" <gi...@apache.org>.
davidhewitt commented on PR #17056:
URL: https://github.com/apache/datafusion/pull/17056#issuecomment-3161163327

   Looking again, it seems that `is_null` has the same problem. Should I update this PR accordingly?


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [PR] fix return field for `is_not_null` expression [datafusion]

Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb merged PR #17056:
URL: https://github.com/apache/datafusion/pull/17056


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org