Skip to content
6 changes: 5 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -55,4 +55,8 @@ http-client.env.json
# Coding agent files (could be symlinks)
.claude
.clinerules
memory-bank
memory-bank

# uv environment config (helps with IDE plugins depending on Python)
pyproject.toml
uv.lock
7 changes: 1 addition & 6 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,6 @@ repos:
hooks:
- id: spotless-format
name: Spotless Format
entry: bash -c './gradlew spotlessApply && git add -u'
language: system
pass_filenames: false
- id: spotless-check
name: Spotless Post-format Check
entry: bash -c './gradlew spotlessCheck'
entry: bash -c './gradlew spotlessApply spotlessCheck && git add -u'
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

small build change, makes precommit faster (check always runs after apply)

language: system
pass_filenames: false
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@
import org.opensearch.sql.ast.tree.Trendline;
import org.opensearch.sql.ast.tree.UnresolvedPlan;
import org.opensearch.sql.ast.tree.Values;
import org.opensearch.sql.ast.tree.args.RareTopNArguments;
import org.opensearch.sql.common.antlr.SyntaxCheckException;
import org.opensearch.sql.data.model.ExprMissingValue;
import org.opensearch.sql.data.type.ExprCoreType;
Expand Down Expand Up @@ -376,10 +377,10 @@ public LogicalPlan visitRareTopN(RareTopN node, AnalysisContext context) {
fields.forEach(
field -> newEnv.define(new Symbol(Namespace.FIELD_NAME, field.toString()), field.type()));

List<Argument> options = node.getArguments();
Integer noOfResults = (Integer) options.get(0).getValue().getValue();
RareTopNArguments options = node.getArguments();

return new LogicalRareTopN(child, node.getCommandType(), noOfResults, fields, groupBys);
return new LogicalRareTopN(
child, node.getCommandType(), options.getNoOfResults(), fields, groupBys);
}

/**
Expand Down
5 changes: 3 additions & 2 deletions core/src/main/java/org/opensearch/sql/ast/dsl/AstDSL.java
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@
import org.opensearch.sql.ast.tree.Trendline;
import org.opensearch.sql.ast.tree.UnresolvedPlan;
import org.opensearch.sql.ast.tree.Values;
import org.opensearch.sql.ast.tree.args.RareTopNArguments;

/** Class of static methods to create specific node instances. */
@UtilityClass
Expand Down Expand Up @@ -494,8 +495,8 @@ public static Head head(UnresolvedPlan input, Integer size, Integer from) {
return new Head(input, size, from);
}

public static List<Argument> defaultTopArgs() {
return exprList(argument("noOfResults", intLiteral(10)));
public static List<Argument> defaultTopRareArgs() {
return new RareTopNArguments().asExprList();
}

public static RareTopN rareTopN(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import org.opensearch.sql.ast.expression.Argument;
import org.opensearch.sql.ast.expression.Field;
import org.opensearch.sql.ast.expression.UnresolvedExpression;
import org.opensearch.sql.ast.tree.args.RareTopNArguments;

/** AST node represent RareTopN operation. */
@Getter
Expand All @@ -29,11 +30,16 @@ public class RareTopN extends UnresolvedPlan {

private UnresolvedPlan child;
private final CommandType commandType;
// arguments: noOfResults: Integer, countField: String, showCount: Boolean
// arguments: noOfResults: Integer, countField: String, showCount: Boolean, percentField: String,
// showPerc: Boolean, useOther: Boolean
private final List<Argument> arguments;
private final List<Field> fields;
private final List<UnresolvedExpression> groupExprList;

public RareTopNArguments getArguments() {
return new RareTopNArguments(arguments);
}

@Override
public RareTopN attach(UnresolvedPlan child) {
this.child = child;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

package org.opensearch.sql.ast.tree.args;

import static org.opensearch.sql.ast.dsl.AstDSL.argument;
import static org.opensearch.sql.ast.dsl.AstDSL.booleanLiteral;
import static org.opensearch.sql.ast.dsl.AstDSL.exprList;
import static org.opensearch.sql.ast.dsl.AstDSL.intLiteral;
import static org.opensearch.sql.ast.dsl.AstDSL.stringLiteral;

import java.util.List;
import lombok.AllArgsConstructor;
import lombok.EqualsAndHashCode;
import lombok.Getter;
import lombok.NoArgsConstructor;
import lombok.ToString;
import org.jetbrains.annotations.TestOnly;
import org.opensearch.sql.ast.expression.Argument;

@ToString
@EqualsAndHashCode
@Getter
@NoArgsConstructor
@AllArgsConstructor
public class RareTopNArguments {
private int noOfResults = 10;
private String countField = "count";
private boolean showCount = true;
private String percentField = "percent";
private boolean showPerc = false;
private boolean useOther = false;

public RareTopNArguments(List<Argument> arguments) {
for (Argument arg : arguments) {
switch (arg.getArgName()) {
case "noOfResults":
noOfResults = (int) arg.getValue().getValue();
break;
case "countField":
countField = (String) arg.getValue().getValue();
if (countField.isBlank()) {
throw new IllegalArgumentException("Illegal count field in top/rare: cannot be blank");
}
break;
case "showCount":
showCount = (boolean) arg.getValue().getValue();
break;
case "percentField":
percentField = (String) arg.getValue().getValue();
if (countField.isBlank()) {
throw new IllegalArgumentException(
"Illegal percent field in top/rare: cannot be blank");
}
break;
case "showPerc":
showPerc = (boolean) arg.getValue().getValue();
break;
case "useOther":
useOther = (boolean) arg.getValue().getValue();
break;
default:
throw new IllegalArgumentException("unknown argument for rare/top: " + arg.getArgName());
}
}
}

public String renderOptions() {
StringBuilder options = new StringBuilder();
if (showCount) {
options.append("countfield='").append(countField).append("' ");
} else {
options.append("showcount=false ");
}
if (showPerc) {
options.append("percfield='").append(percentField).append("' ");
} else {
options.append("showperc=false ");
}
if (useOther) {
options.append("useother=true ");
}
return options.toString();
}

@TestOnly
public List<Argument> asExprList() {
return exprList(
argument("noOfResults", intLiteral(10)),
argument("countField", stringLiteral("count")),
argument("showCount", booleanLiteral(true)),
argument("percentField", stringLiteral("percent")),
argument("showPerc", booleanLiteral(false)),
argument("useOther", booleanLiteral(false)));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@
import org.opensearch.sql.ast.expression.AllFields;
import org.opensearch.sql.ast.expression.AllFieldsExcludeMeta;
import org.opensearch.sql.ast.expression.Argument;
import org.opensearch.sql.ast.expression.Argument.ArgumentMap;
import org.opensearch.sql.ast.expression.Field;
import org.opensearch.sql.ast.expression.Function;
import org.opensearch.sql.ast.expression.Let;
Expand Down Expand Up @@ -124,6 +123,7 @@
import org.opensearch.sql.ast.tree.UnresolvedPlan;
import org.opensearch.sql.ast.tree.Values;
import org.opensearch.sql.ast.tree.Window;
import org.opensearch.sql.ast.tree.args.RareTopNArguments;
import org.opensearch.sql.calcite.plan.OpenSearchConstants;
import org.opensearch.sql.calcite.utils.BinUtils;
import org.opensearch.sql.calcite.utils.JoinAndLookupUtils;
Expand Down Expand Up @@ -1577,13 +1577,19 @@ public RelNode visitKmeans(Kmeans node, CalcitePlanContext context) {
public RelNode visitRareTopN(RareTopN node, CalcitePlanContext context) {
visitChildren(node, context);

ArgumentMap arguments = ArgumentMap.of(node.getArguments());
String countFieldName = (String) arguments.get("countField").getValue();
RareTopNArguments arguments = node.getArguments();
String countFieldName = arguments.getCountField();
if (context.relBuilder.peek().getRowType().getFieldNames().contains(countFieldName)) {
throw new IllegalArgumentException(
"Field `"
"The top/rare output field `"
+ countFieldName
+ "` is existed, change the count field by setting countfield='xyz'");
+ "` already exists. Suggestion: change the count field by adding countfield='xyz'");
}
if (arguments.isUseOther()) {
throw new CalciteUnsupportedException("`useother` is currently unsupported. (Coming soon)");
}
if (arguments.isShowPerc()) {
throw new CalciteUnsupportedException("`showperc` is currently unsupported. (Coming soon)");
}

// 1. group the group-by list + field list and add a count() aggregation
Expand Down Expand Up @@ -1616,14 +1622,13 @@ public RelNode visitRareTopN(RareTopN node, CalcitePlanContext context) {
context.relBuilder.alias(rowNumberWindowOver, ROW_NUMBER_COLUMN_NAME));

// 3. filter row_number() <= k in each partition
Integer N = (Integer) arguments.get("noOfResults").getValue();
context.relBuilder.filter(
context.relBuilder.lessThanOrEqual(
context.relBuilder.field(ROW_NUMBER_COLUMN_NAME), context.relBuilder.literal(N)));
context.relBuilder.field(ROW_NUMBER_COLUMN_NAME),
context.relBuilder.literal(arguments.getNoOfResults())));

// 4. project final output. the default output is group by list + field list
Boolean showCount = (Boolean) arguments.get("showCount").getValue();
if (showCount) {
if (arguments.isShowCount()) {
context.relBuilder.projectExcept(context.relBuilder.field(ROW_NUMBER_COLUMN_NAME));
} else {
context.relBuilder.projectExcept(
Expand Down
2 changes: 2 additions & 0 deletions ppl/src/main/antlr/OpenSearchPPLLexer.g4
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,8 @@ ANOMALY_SCORE_THRESHOLD: 'ANOMALY_SCORE_THRESHOLD';
APPEND: 'APPEND';
COUNTFIELD: 'COUNTFIELD';
SHOWCOUNT: 'SHOWCOUNT';
SHOWPERC: 'SHOWPERC';
PERCENTFIELD: 'PERCENTFIELD';
LIMIT: 'LIMIT';
USEOTHER: 'USEOTHER';
INPUT: 'INPUT';
Expand Down
7 changes: 5 additions & 2 deletions ppl/src/main/antlr/OpenSearchPPLParser.g4
Original file line number Diff line number Diff line change
Expand Up @@ -301,12 +301,13 @@ logSpanValue
: LOG_WITH_BASE # logWithBaseSpan
;

// TODO support arbitrary argument ordering
topCommand
: TOP (number = integerLiteral)? (COUNTFIELD EQUAL countfield = stringLiteral)? (SHOWCOUNT EQUAL showcount = booleanLiteral)? fieldList (byClause)?
: TOP (number = integerLiteral)? (COUNTFIELD EQUAL countfield = stringLiteral)? (SHOWCOUNT EQUAL showcount = booleanLiteral)? (PERCENTFIELD EQUAL percentfield = stringLiteral)? (SHOWPERC EQUAL showperc = booleanLiteral)? (USEOTHER EQUAL useother = booleanLiteral)? fieldList (byClause)?
;

rareCommand
: RARE (number = integerLiteral)? (COUNTFIELD EQUAL countfield = stringLiteral)? (SHOWCOUNT EQUAL showcount = booleanLiteral)? fieldList (byClause)?
: RARE (number = integerLiteral)? (COUNTFIELD EQUAL countfield = stringLiteral)? (SHOWCOUNT EQUAL showcount = booleanLiteral)? (PERCENTFIELD EQUAL percentfield = stringLiteral)? (SHOWPERC EQUAL showperc = booleanLiteral)? (USEOTHER EQUAL useother = booleanLiteral)? fieldList (byClause)?
;

grokCommand
Expand Down Expand Up @@ -1462,6 +1463,8 @@ searchableKeyWord
| ANOMALY_SCORE_THRESHOLD
| COUNTFIELD
| SHOWCOUNT
| SHOWPERC
| PERCENTFIELD
| PATH
| INPUT
| OUTPUT
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,23 +127,47 @@ public static List<Argument> getArgumentList(SortFieldContext ctx) {
: new Argument("type", new Literal(null, DataType.NULL)));
}

/**
* Helper: dedupe list generation logic for top & rare. Helps ensure both commands consistently
* support the same args.
*/
private static List<Argument> topRareArgList(
ParserRuleContext number,
ParserRuleContext countfield,
ParserRuleContext showcount,
ParserRuleContext percentfield,
ParserRuleContext showperc,
ParserRuleContext useother) {
return Arrays.asList(
number != null
? new Argument("noOfResults", getArgumentValue(number))
: new Argument("noOfResults", new Literal(10, DataType.INTEGER)),
countfield != null
? new Argument("countField", getArgumentValue(countfield))
: new Argument("countField", new Literal("count", DataType.STRING)),
showcount != null
? new Argument("showCount", getArgumentValue(showcount))
: new Argument("showCount", new Literal(true, DataType.BOOLEAN)),
percentfield != null
? new Argument("percentField", getArgumentValue(percentfield))
: new Argument("percentField", new Literal("percent", DataType.STRING)),
showperc != null
? new Argument("showPerc", getArgumentValue(showperc))
: new Argument("showPerc", new Literal(percentfield != null, DataType.BOOLEAN)),
useother != null
? new Argument("useOther", getArgumentValue(useother))
: new Argument("useOther", new Literal(false, DataType.BOOLEAN)));
}

/**
* Get list of {@link Argument}.
*
* @param ctx TopCommandContext instance
* @return the list of arguments fetched from the top command
*/
public static List<Argument> getArgumentList(TopCommandContext ctx) {
return Arrays.asList(
ctx.number != null
? new Argument("noOfResults", getArgumentValue(ctx.number))
: new Argument("noOfResults", new Literal(10, DataType.INTEGER)),
ctx.countfield != null
? new Argument("countField", getArgumentValue(ctx.countfield))
: new Argument("countField", new Literal("count", DataType.STRING)),
ctx.showcount != null
? new Argument("showCount", getArgumentValue(ctx.showcount))
: new Argument("showCount", new Literal(true, DataType.BOOLEAN)));
return topRareArgList(
ctx.number, ctx.countfield, ctx.showcount, ctx.percentfield, ctx.showperc, ctx.useother);
}

/**
Expand All @@ -153,16 +177,8 @@ public static List<Argument> getArgumentList(TopCommandContext ctx) {
* @return the list of argument with default number of results for the rare command
*/
public static List<Argument> getArgumentList(RareCommandContext ctx) {
return Arrays.asList(
ctx.number != null
? new Argument("noOfResults", getArgumentValue(ctx.number))
: new Argument("noOfResults", new Literal(10, DataType.INTEGER)),
ctx.countfield != null
? new Argument("countField", getArgumentValue(ctx.countfield))
: new Argument("countField", new Literal("count", DataType.STRING)),
ctx.showcount != null
? new Argument("showCount", getArgumentValue(ctx.showcount))
: new Argument("showCount", new Literal(true, DataType.BOOLEAN)));
return topRareArgList(
ctx.number, ctx.countfield, ctx.showcount, ctx.percentfield, ctx.showperc, ctx.useother);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import org.opensearch.sql.ast.expression.AllFieldsExcludeMeta;
import org.opensearch.sql.ast.expression.And;
import org.opensearch.sql.ast.expression.Argument;
import org.opensearch.sql.ast.expression.Argument.ArgumentMap;
import org.opensearch.sql.ast.expression.Between;
import org.opensearch.sql.ast.expression.Case;
import org.opensearch.sql.ast.expression.Cast;
Expand Down Expand Up @@ -89,6 +88,7 @@
import org.opensearch.sql.ast.tree.UnresolvedPlan;
import org.opensearch.sql.ast.tree.Values;
import org.opensearch.sql.ast.tree.Window;
import org.opensearch.sql.ast.tree.args.RareTopNArguments;
import org.opensearch.sql.common.setting.Settings;
import org.opensearch.sql.common.utils.StringUtils;
import org.opensearch.sql.planner.logical.LogicalAggregation;
Expand Down Expand Up @@ -353,17 +353,13 @@ public String visitWindow(Window node, String context) {
/** Build {@link LogicalRareTopN}. */
@Override
public String visitRareTopN(RareTopN node, String context) {
final String child = node.getChild().get(0).accept(this, context);
ArgumentMap arguments = ArgumentMap.of(node.getArguments());
Integer noOfResults = (Integer) arguments.get("noOfResults").getValue();
String countField = (String) arguments.get("countField").getValue();
Boolean showCount = (Boolean) arguments.get("showCount").getValue();
final String child = node.getChild().getFirst().accept(this, context);
RareTopNArguments arguments = node.getArguments();
Integer noOfResults = arguments.getNoOfResults();
String fields = visitFieldList(node.getFields());
String group = visitExpressionList(node.getGroupExprList());
String options =
isCalciteEnabled(settings)
? StringUtils.format("countield='%s' showcount=%s ", countField, showCount)
: "";
String options = isCalciteEnabled(settings) ? arguments.renderOptions() : "";

return StringUtils.format(
"%s | %s %d %s%s",
child,
Expand Down
Loading
Loading