[GitHub] [flink] tsreaper commented on a diff in pull request #21759: [FLINK-29237][table] Remove RexSimplify from Flink code, SearchOperator code generation for RexUnknown.nullAs

["tsreaper (via GitHub)" <gi...@apache.org>]

tsreaper commented on code in PR #21759:
URL: https://github.com/apache/flink/pull/21759#discussion_r1096901589


##########
flink-table/flink-table-planner/src/main/scala/org/apache/flink/table/planner/codegen/calls/SearchOperatorGen.scala:
##########
@@ -96,7 +96,9 @@ object SearchOperatorGen {
            |boolean $nullTerm = true;
            |if (!${needle.nullTerm}) {
            |  $resultTerm = $negation$setTerm.contains(${needle.resultTerm});
-           |  $nullTerm = !$resultTerm && $setTerm.containsNull();
+           |  $nullTerm = false;
+           |} else {
+           |  $resultTerm = $setTerm.containsNull();

Review Comment:
   > Please also show us where you get the information with your changes
   
   Are you sure that `sarg.nullAs == RexUnknownAs.TRUE` is always the case? Is it possible that `sarg.nullAs == RexUnknownAs.FALSE` or even `sarg.nullAs == RexUnknownAs.UNKNOWN`? Where do you learn the information?



##########
flink-table/flink-table-planner/src/test/scala/org/apache/flink/table/planner/runtime/batch/sql/CalcITCase.scala:
##########
@@ -1835,6 +1938,18 @@ class CalcITCase extends BatchTestBase {
     checkResult(
       "SELECT IF(a = '' OR a IS NULL, 'a', 'b') FROM MyTable",
       Seq(row('a'), row('b'), row('a')))
+    checkResult(
+      "SELECT IF(a IN ('', ' ') OR a IS NULL, 'a', 'b') FROM MyTable",
+      Seq(row('a'), row('b'), row('a')))
+    checkResult(
+      "SELECT IF(a IN ('', ' ') OR a IS NOT NULL, 'a', 'b') FROM MyTable",
+      Seq(row('a'), row('a'), row('b')))
+    checkResult(
+      "SELECT IF(a NOT IN ('', ' ') OR a IS NULL, 'a', 'b') FROM MyTable",
+      Seq(row('b'), row('a'), row('a')))
+    checkResult(
+      "SELECT IF(a NOT IN ('', ' ') OR a IS NOT NULL, 'a', 'b') FROM MyTable",
+      Seq(row('a'), row('a'), row('b')))

Review Comment:
   Results of the 2nd and the 4th test are the same. You might need to add more test data to make them different.



-- 
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: issues-unsubscribe@flink.apache.org

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