tsreaper commented on code in PR #21759:
URL: https://github.com/apache/flink/pull/21759#discussion_r1098519240
##########
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:
You didn't set `$nullTerm`, so the result will always be `UNKNOWN`.
I actually expect something like below to make the code clearer.
```scala
val isNullCode = sarg.nullAs match {
case RexUnknownAs.TRUE =>
s"""
|$resultTerm = true;
|$nullTerm = false;
|""".stripMargin
case RexUnknownAs.FALSE =>
s"""
|$resultTerm = false;
|$nullTerm = false;
|""".stripMargin
case RexUnknownAs.UNKNOWN =>
s"""
|$resultTerm = false;
|$nullTerm = true;
|""".stripMargin
}
// ...
|} else {
| $isNullCode
```
Also add code comments on what `sarg.nullAs` means. It would be better to
add the links you mentioned above in the code comments.
##########
flink-table/flink-table-planner/src/main/scala/org/apache/flink/table/planner/codegen/calls/SearchOperatorGen.scala:
##########
@@ -112,7 +114,7 @@ object SearchOperatorGen {
var rangeChecks: Seq[GeneratedExpression] =
sarg.rangeSet.asRanges.asScala.toSeq
.map(RangeSets.map(_, rangeToExpression))
- if (sarg.containsNull) {
+ if (sarg.nullAs == RexUnknownAs.TRUE) {
rangeChecks =
Seq(generateIsNull(target, new
BooleanType(target.resultType.isNullable))) ++ rangeChecks
}
Review Comment:
I think it would be better to first check if `needle` is null, then handle
it differently. Just like what we do with point searches.
--
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]
