clintropolis commented on code in PR #15034:
URL: https://github.com/apache/druid/pull/15034#discussion_r1343374958
##########
processing/src/main/java/org/apache/druid/math/expr/BuiltInExprMacros.java:
##########
@@ -25,20 +25,28 @@
import javax.annotation.Nullable;
import java.util.Collections;
import java.util.List;
+import java.util.Optional;
import java.util.stream.Collectors;
public class BuiltInExprMacros
{
public static class ComplexDecodeBase64ExprMacro implements
ExprMacroTable.ExprMacro
{
public static final String NAME = "complex_decode_base64";
+ public static final String ALIAS_NAME = "decode_base64_complex";
@Override
public String name()
{
return NAME;
}
+ @Override
+ public Optional<String> alias()
Review Comment:
I'm a bit worried this might be a bit too limiting since there can only be a
single alias, unlike the SQL layer which can have any number of alias for the
same function.
Additionally, I think there might be a bit more work to do here in terms of
making the error messaging correct. Using this function as an example, the
error messaging is going to be using the output of `name()` rather than the
alias, so the error messaging for `decode_base64_complex` is going to be
reporting `complex_decode_base64`, which I think might be confusing for users.
We might want to consider doing aliasing similar to how it is done at the SQL
layer, where we have some `AliasNamedFunction` class that overrides the
`name()` method with the alias and then delegates everything else to the
underlying expression which is being overridden, though it might take separate
classes for `ExprMacroFunctionExpr`, `FunctionExpr`, and `ApplyFunctionExpr`.
Depending on how these are registered with the function tables, this might also
solve the multi-alias problem this current approach has, and future proofs
against cases where we need to rename functions more than one time.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
