Skip to content

Add scalar min/max to BuiltinFunctionName #4967

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
LantaoJin merged 1 commit into from
Dec 18, 2025
Merged

Add scalar min/max to BuiltinFunctionName #4967

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion core/src/main/java/org/opensearch/sql/calcite/CalciteRelNodeVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2905,7 +2905,7 @@ private RelNode rankByColumnSplit(
*/
private AggCall buildAggCall(RelBuilder relBuilder, String aggFunctionName, RexNode node) {
BuiltinFunctionName aggFunction =
BuiltinFunctionName.of(aggFunctionName)
BuiltinFunctionName.ofAggregation(aggFunctionName)
Copy link
Collaborator

@yuancu yuancu Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please kindly add a javadoc on BuiltinFunctionName.of to instruct future developers to use BuiltinFunctionName.ofAggregation for aggregation functions?

.orElseThrow(
() ->
new IllegalArgumentException(
Expand Down
2 changes: 2 additions & 0 deletions core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionName.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ public enum BuiltinFunctionName {
SIN(FunctionName.of("sin")),
TAN(FunctionName.of("tan")),
SPAN(FunctionName.of("span")),
SCALAR_MAX(FunctionName.of("scalar_max")),
SCALAR_MIN(FunctionName.of("scalar_min")),

/** Binning Functions. */
SPAN_BUCKET(FunctionName.of("span_bucket")),
Expand Down
8 changes: 5 additions & 3 deletions core/src/main/java/org/opensearch/sql/expression/function/PPLFuncImpTable.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,8 @@
import static org.opensearch.sql.expression.function.BuiltinFunctionName.RINT;
import static org.opensearch.sql.expression.function.BuiltinFunctionName.ROUND;
import static org.opensearch.sql.expression.function.BuiltinFunctionName.RTRIM;
import static org.opensearch.sql.expression.function.BuiltinFunctionName.SCALAR_MAX;
import static org.opensearch.sql.expression.function.BuiltinFunctionName.SCALAR_MIN;
import static org.opensearch.sql.expression.function.BuiltinFunctionName.SECOND;
import static org.opensearch.sql.expression.function.BuiltinFunctionName.SECOND_OF_MINUTE;
import static org.opensearch.sql.expression.function.BuiltinFunctionName.SEC_TO_TIME;
Expand Down Expand Up @@ -868,9 +870,9 @@ void populate() {
registerOperator(INTERNAL_REGEXP_REPLACE_5, SqlLibraryOperators.REGEXP_REPLACE_5);
registerOperator(INTERNAL_TRANSLATE3, SqlLibraryOperators.TRANSLATE3);

// Register eval functions for PPL max() and min() calls
registerOperator(MAX, PPLBuiltinOperators.SCALAR_MAX);
registerOperator(MIN, PPLBuiltinOperators.SCALAR_MIN);
// Register eval functions for PPL scalar max() and min() calls
registerOperator(SCALAR_MAX, PPLBuiltinOperators.SCALAR_MAX);
registerOperator(SCALAR_MIN, PPLBuiltinOperators.SCALAR_MIN);

// Register PPL UDF operator
registerOperator(COSH, PPLBuiltinOperators.COSH);
Expand Down
9 changes: 6 additions & 3 deletions ppl/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,13 +82,15 @@ public class AstExpressionBuilder extends OpenSearchPPLParserBaseVisitor<Unresol

private static final int DEFAULT_TAKE_FUNCTION_SIZE_VALUE = 10;

/** The function name mapping between fronted and core engine. */
private static final Map<String, String> FUNCTION_NAME_MAPPING =
/** The eval function name mapping between fronted and core engine. */
private static final Map<String, String> EVAL_FUNCTION_NAME_MAPPING =
new ImmutableMap.Builder<String, String>()
.put("isnull", IS_NULL.getName().getFunctionName())
.put("isnotnull", IS_NOT_NULL.getName().getFunctionName())
.put("regex_match", REGEXP_MATCH.getName().getFunctionName()) // compatible with old one
.put("regexp_replace", REPLACE.getName().getFunctionName())
.put("max", SCALAR_MAX.getName().getFunctionName()) // this is scalar max
.put("min", SCALAR_MIN.getName().getFunctionName()) // this is scalar min
.build();

private final AstBuilder astBuilder;
Expand Down Expand Up @@ -439,7 +441,8 @@ public UnresolvedExpression visitCaseFunctionCall(
public UnresolvedExpression visitEvalFunctionCall(EvalFunctionCallContext ctx) {
final String functionName = ctx.evalFunctionName().getText();
final String mappedName =
FUNCTION_NAME_MAPPING.getOrDefault(functionName.toLowerCase(Locale.ROOT), functionName);
EVAL_FUNCTION_NAME_MAPPING.getOrDefault(
functionName.toLowerCase(Locale.ROOT), functionName);

// Rewrite sum and avg functions to arithmetic expressions
if (SUM.getName().getFunctionName().equalsIgnoreCase(mappedName)
Expand Down
Loading