rubenada commented on code in PR #5189:
URL: https://github.com/apache/calcite/pull/5189#discussion_r3806510763
##########
core/src/main/java/org/apache/calcite/adapter/enumerable/StrictAggImplementor.java:
##########
@@ -163,9 +163,7 @@ protected abstract void implementNotNullAdd(AggContext info,
return EnumUtils.convert(
implementNotNullResult(info, result), info.returnType());
}
- String tmpName = result.accumulator().isEmpty()
- ? "ar"
- : (result.accumulator().get(0) + "$Res");
+ String tmpName = result.accumulator().isEmpty() ? "ar" : "acc$Res";
Review Comment:
This was actually a latent bug unveiled by this patch.
For instance TpchTests would fail now on the new checkArgument with "field
name should be a valid java identifier" since the old code could include a dot
on the identifier (which is incorrect).
The problem was (by chance) hidden because Janino code generator inlined the
expression directly, without actually using the problematic identifier; but the
bug was there, latent.
--
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]
