Copilot commented on code in PR #7328:
URL: https://github.com/apache/ignite-3/pull/7328#discussion_r2651228443
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/RexUtils.java:
##########
@@ -1247,6 +1249,48 @@ public static IntSet notNullKeys(List<RexNode> row) {
return keys;
}
+ /**
+ * Expand SEARCH/SARG operator with preceding NULLs check for nullable
input operator. This allows to eliminate NULL checks for every
Review Comment:
The documentation comment says "NULLs check" but it should be "NULL check"
or "NULL checks" for clarity and consistency. The term "NULLs check" is awkward
phrasing.
```suggestion
* Expand SEARCH/SARG operator with a preceding NULL check for nullable
input operator. This allows to eliminate NULL checks for every
```
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/util/RexUtils.java:
##########
@@ -1247,6 +1249,48 @@ public static IntSet notNullKeys(List<RexNode> row) {
return keys;
}
+ /**
+ * Expand SEARCH/SARG operator with preceding NULLs check for nullable
input operator. This allows to eliminate NULL checks for every
+ * argument of SARG and reduces generated bytecode.
+ */
+ public static RexNode expandSearchNullable(RexBuilder rexBuilder,
@Nullable RexProgram program, RexCall call) {
+ assert call.getOperator() == SqlStdOperatorTable.SEARCH : "Unexpected
operator: " + call.getOperator();
+
+ RexNode op0 = call.getOperands().get(0);
+ RexNode op1 = call.getOperands().get(1);
+
+ if (!op0.getType().isNullable()) {
+ return RexUtil.expandSearch(rexBuilder, program, call);
+ }
+
+ // Dereference local variable.
+ while (op1 instanceof RexLocalRef) {
+ op1 = requireNonNull(program,
"program").getExprList().get(((RexSlot) op1).getIndex());
+ }
+
+ assert op1 instanceof RexLiteral : "Unexpected operand class: " + op1;
+
+ Sarg<?> arg = ((RexLiteral) op1).getValueAs(Sarg.class);
+
+ RexNode nullAs =
+ arg.nullAs == RexUnknownAs.TRUE || arg.nullAs ==
RexUnknownAs.FALSE ? rexBuilder.makeLiteral(arg.nullAs.toBoolean())
+ :
rexBuilder.makeNullLiteral(rexBuilder.getTypeFactory().createSqlType(SqlTypeName.BOOLEAN));
+
+ RexNode expandedSearch = RexUtil.expandSearch(rexBuilder, program,
+ rexBuilder.makeCall(call.getOperator(),
rexBuilder.makeNotNull(op0), op1));
+
+ return rexBuilder.makeCall(SqlStdOperatorTable.CASE,
rexBuilder.makeCall(SqlStdOperatorTable.IS_NULL, op0), nullAs, expandedSearch);
+ }
+
+ /**
+ * Traverse {@code node} and expand all SEARCH/SARG operators (with
preceding NULLs check).
Review Comment:
The comment says "NULLs check" but it should be "NULL checks" for
consistency with standard English grammar. Similarly in line 1286.
--
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]
