mihaibudiu commented on code in PR #4889:
URL: https://github.com/apache/calcite/pull/4889#discussion_r3089471386
##########
core/src/main/java/org/apache/calcite/adapter/enumerable/MatchUtils.java:
##########
@@ -27,21 +27,22 @@ private MatchUtils() {
}
/**
- * Returns the row with the highest index whose corresponding symbol
matches, null otherwise.
+ * Returns the row with the highest index whose corresponding symbol matches,
+ * row with defaultIndex otherwise.
*
* @param symbol Target Symbol
* @param rows List of passed rows
* @param symbols Corresponding symbols to rows
- * @return index or -1
+ * @return index or defaultIndex
*/
- public static <E> int lastWithSymbol(String symbol, List<E> rows,
List<String> symbols,
- int startIndex) {
+ public static <E> int lastWithSymbolOrDefault(String symbol, List<E> rows,
List<String> symbols,
Review Comment:
good question, why have rows if they are not needed? It's not like you need
to implement a specific interface.
Since you are renaming a public function (which may not be ok), maybe you
can just use a new one.
##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##########
@@ -4173,43 +4172,27 @@ private static class LastImplementor implements
MatchImplementor {
final String alpha = ((RexPatternFieldRef)
call.getOperands().get(0)).getAlpha();
- // TODO: verify if the variable is needed
- @SuppressWarnings("unused")
- final BinaryExpression lastIndex =
- Expressions.subtract(
- Expressions.call(rows, BuiltInMethod.COLLECTION_SIZE.method),
- Expressions.constant(1));
-
// Just take the last one, if exists
if ("*".equals(alpha)) {
setInputGetterIndex(translator, i);
- // Important, unbox the node / expression to avoid NullAs.NOT_POSSIBLE
- final RexPatternFieldRef ref = (RexPatternFieldRef) node;
- final RexPatternFieldRef newRef =
- new RexPatternFieldRef(ref.getAlpha(),
- ref.getIndex(),
- translator.typeFactory.createTypeWithNullability(ref.getType(),
- true));
- final Expression expression = translator.translate(newRef,
NullAs.NULL);
- setInputGetterIndex(translator, null);
- return expression;
} else {
// Alpha != "*" so we have to search for a specific one to find and
use that, if found
+ // otherwise pick the last one
setInputGetterIndex(translator,
- Expressions.call(BuiltInMethod.MATCH_UTILS_LAST_WITH_SYMBOL.method,
- Expressions.constant(alpha), rows, symbols, i));
-
- // Important, unbox the node / expression to avoid NullAs.NOT_POSSIBLE
- final RexPatternFieldRef ref = (RexPatternFieldRef) node;
- final RexPatternFieldRef newRef =
- new RexPatternFieldRef(ref.getAlpha(),
- ref.getIndex(),
- translator.typeFactory.createTypeWithNullability(ref.getType(),
- true));
- final Expression expression = translator.translate(newRef,
NullAs.NULL);
- setInputGetterIndex(translator, null);
- return expression;
+
Expressions.call(BuiltInMethod.MATCH_UTILS_LAST_WITH_SYMBOL_OR_DEFAULT.method,
Review Comment:
Are the last two arguments always the same? Then why are they needed?
Does that function have any other callers?
--
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]
