Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ public enum CommandType {
public enum Option {
countField,
showCount,
percentField,
showPerc,
useNull,
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,8 @@ public class CalciteRelNodeVisitor extends AbstractNodeVisitor<RelNode, CalciteP
private final MapPathPreMaterializer mapPathMaterializer;
private final ForeachPlanner foreachPlanner;

private static final int PERCENT_DECIMAL_PLACES = 6;

public CalciteRelNodeVisitor(DataSourceService dataSourceService) {
this.rexVisitor = new CalciteRexNodeVisitor(this);
this.aggVisitor = new CalciteAggCallVisitor(rexVisitor);
Expand Down Expand Up @@ -3264,6 +3266,45 @@ public RelNode visitRareTopN(RareTopN node, CalcitePlanContext context) {
orderKeys.add(countField);
orderKeys.addAll(tieBreakKeys);

// 3. compute percentage if showperc=true
Boolean showPerc = (Boolean) argumentMap.get(RareTopN.Option.showPerc.name()).getValue();
if (showPerc) {
String percentFieldName =
(String) argumentMap.get(RareTopN.Option.percentField.name()).getValue();

if (context.relBuilder.peek().getRowType().getFieldNames().contains(percentFieldName)) {
throw new IllegalArgumentException(
"Field `"
+ percentFieldName
+ "` already exists in the output. Consider renaming with percentfield='xyz'");
Comment on lines +3242 to +3245

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

suggestion: Would it be better user experience if we provided a new percent field automatically?

e.g. generate percent0, percent1, etc. Joins already do this with join IDs (id -> id0, etc).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure, will remove the check.

}
RexNode totalWindowOver =
Comment thread
AjimelecGonzalez marked this conversation as resolved.
PlanUtils.makeOver(
context,
BuiltinFunctionName.SUM,
context.relBuilder.field(countFieldName),
List.of(),
partitionKeys,
List.of(),
WindowFrame.rowsUnbounded());

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

issue: This has a correctness issue for high-cardinality fields because the total is based on the returned buckets.

The current impl works well for low-cardinality fields, but for high cardinality we get a truncated bucket set. I'm worried about long-tail types of top-N cases (e.g. "top 10 endpoints" from a service with many assets). rare would be much more affected in that case.

In principle we should be using the sum_other_doc_count value that's returned by aggregations if it's available: https://docs.opensearch.org/latest/aggregations/bucket/terms/#example-response. But fixing may be complex depending on the exact conditions where that value is present, I don't know them off the top but it may only apply to term aggregations and not composite aggs, and it means percent aggregations need to be finalized at the coordinator. If it's not available then the "right" way is to submit a whole separate _count request but that'll complicate query flow.

If it's a lot to patch, I think we can leave this behavior, but should document the limitation. You can manually work around it by using a subquery or something like eventstats to work with stats count() computed ahead of time. (Alternatively, if such a method exists, we could rewrite the query plan to be equivalent to it.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Will look into it and document, I believe that the using the sum_other_doc_count value would be the best choice here but will have to deep dive. For now I will add the limitations to the md file for top and rare commands.


RexNode hundred = context.relBuilder.literal(new BigDecimal("100.0"));
RexNode countCast =
context.relBuilder.cast(context.relBuilder.field(countFieldName), SqlTypeName.DOUBLE);
RexNode totalCast = context.relBuilder.cast(totalWindowOver, SqlTypeName.DOUBLE);
RexNode numerator = context.relBuilder.call(SqlStdOperatorTable.MULTIPLY, hundred, countCast);
RexNode percValue = context.relBuilder.call(SqlStdOperatorTable.DIVIDE, numerator, totalCast);

// Round the percent value 6 decimal places
RexNode roundedPerc =
context.relBuilder.call(
SqlStdOperatorTable.ROUND,
percValue,
context.relBuilder.literal(PERCENT_DECIMAL_PLACES));

context.relBuilder.projectPlus(context.relBuilder.alias(roundedPerc, percentFieldName));
}

RexNode rowNumberWindowOver =
PlanUtils.makeOver(
context,
Expand All @@ -3276,14 +3317,14 @@ public RelNode visitRareTopN(RareTopN node, CalcitePlanContext context) {
context.relBuilder.projectPlus(
context.relBuilder.alias(rowNumberWindowOver, ROW_NUMBER_COLUMN_FOR_RARE_TOP));

// 3. filter row_number() <= k in each partition
// 4. filter row_number() <= k in each partition
int k = node.getNoOfResults();
context.relBuilder.filter(
context.relBuilder.lessThanOrEqual(
context.relBuilder.field(ROW_NUMBER_COLUMN_FOR_RARE_TOP),
context.relBuilder.literal(k)));

// 4. project final output. the default output is group by list + field list
// 5. project final output: group by list + field list, optionally count and percent
Boolean showCount = (Boolean) argumentMap.get(RareTopN.Option.showCount.name()).getValue();
if (showCount) {
context.relBuilder.projectExcept(context.relBuilder.field(ROW_NUMBER_COLUMN_FOR_RARE_TOP));
Expand Down
51 changes: 48 additions & 3 deletions docs/user/ppl/cmd/rare.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ The `rare` command supports the following parameters.
| --- | --- | --- |
| `<field-list>` | Required | A comma-delimited list of field names. |
| `<by-clause>` | Optional | One or more fields to group the results by. |
| `rare-options` | Optional | Additional options for controlling output: <br> - `showcount`: Whether to create a field in the output containing the frequency count for each combination of values. Default is `true`. <br> - `countfield`: The name of the field that contains the count. Default is `count`. <br> - `usenull`: Whether to output null values. Default is the value of `plugins.ppl.syntax.legacy.preferred`. |
| `rare-options` | Optional | Additional options for controlling output: <br> - `showcount`: Whether to create a field in the output containing the frequency count for each combination of values. Default is `true`. <br> - `countfield`: The name of the field that contains the count. Default is `count`. <br> - `percentfield`: The name of the field that contains the percentage. Default is `percent`. <br> - `showperc`: Whether to create a field in the output containing the percentage of each value's count relative to the total. Default is `false`. <br> - `usenull`: Whether to output null values. Default is the value of `plugins.ppl.syntax.legacy.preferred`. |

## Example 1: Finding the least common values without showing counts

Expand Down Expand Up @@ -125,7 +125,52 @@ fetched rows / total rows = 4/4
+--------------+-----+
```

## Example 5: Specifying null value handling
## Example 5: Displaying percentages

The following query finds the least common severity levels and shows what percentage each represents:

```ppl
source=otellogs
| rare showperc=true severityText
```

The query returns the following results:

```text
fetched rows / total rows = 4/4
+--------------+-------+---------+
| severityText | count | percent |
|--------------+-------+---------|
| DEBUG | 3 | 15.0 |
| WARN | 4 | 20.0 |
| INFO | 6 | 30.0 |
| ERROR | 7 | 35.0 |
+--------------+-------+---------+
```

## Example 6: Customizing the percent field name
The following query uses `percentfield` to rename the percentage column from the default `percent` to `pct`:

```ppl
source=otellogs
| rare showperc=true percentfield='pct' severityText
```

The query returns the following results:

```text
fetched rows / total rows = 4/4
+--------------+-------+------+
| severityText | count | pct |
|--------------+-------+------|
| DEBUG | 3 | 15.0 |
| WARN | 4 | 20.0 |
| INFO | 6 | 30.0 |
| ERROR | 7 | 35.0 |
+--------------+-------+------+
```

## Example 7: Specifying null value handling

The following query uses `usenull=false` to exclude null values:

Expand Down Expand Up @@ -166,4 +211,4 @@ fetched rows / total rows = 4/4
| @opentelemetry/instrumentation-http | 2 |
| null | 16 |
+-----------------------------------------------------------------------------+-------+
```
```
49 changes: 47 additions & 2 deletions docs/user/ppl/cmd/top.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ The `top` command supports the following parameters.
| Parameter | Required/Optional | Description |
| --- | --- | --- |
| `<N>` | Optional | The number of results to return. Default is `10`. |
| `top-options` | Optional | `showcount`: Whether to create a field in the output that represents a count of the tuple of values. Default is `true`.<br>`countfield`: The name of the field that contains the count. Default is `count`.<br>`usenull`: Whether to output `null` values. Default is the value of `plugins.ppl.syntax.legacy.preferred`. |
| `top-options` | Optional | `showcount`: Whether to create a field in the output that represents a count of the tuple of values. Default is `true`.<br>`countfield`: The name of the field that contains the count. Default is `count`.<br>`percentfield`: The name of the field that contains the percentage. Default is `percent`.<br>`showperc`: Whether to create a field in the output that represents the percentage of the count relative to the total. Default is `false`.<br>`usenull`: Whether to output `null` values. Default is the value of `plugins.ppl.syntax.legacy.preferred`. |
| `<field-list>` | Required | A comma-delimited list of field names. |
| `<by-clause>` | Optional | One or more fields to group the results by. |

Expand Down Expand Up @@ -139,7 +139,52 @@ fetched rows / total rows = 7/7
+----------------------------------+--------------+
```

## Example 6: Specifying null value handling
## Example 6: Displaying percentages

The following query finds the most common severity levels and shows the percentage each represents:

```ppl
source=otellogs
| top showperc=true severityText
```

The query returns the following results:

```text
fetched rows / total rows = 4/4
+--------------+-------+---------+
| severityText | count | percent |
|--------------+-------+---------|
| ERROR | 7 | 35.0 |
| INFO | 6 | 30.0 |
| WARN | 4 | 20.0 |
| DEBUG | 3 | 15.0 |
+--------------+-------+---------+
```

## Example 7: Customizing the percent field name
The following query uses `percentfield` to rename the percentage column from the default `percent` to `pct`:

```ppl
source=otellogs
| top showperc=true percentfield='pct' severityText
```

The query returns the following results:

```text
fetched rows / total rows = 4/4
+--------------+-------+------+
| severityText | count | pct |
|--------------+-------+------|
| ERROR | 7 | 35.0 |
| INFO | 6 | 30.0 |
| WARN | 4 | 20.0 |
| DEBUG | 3 | 15.0 |
+--------------+-------+------+
```

## Example 8: Specifying null value handling

The following query specifies `usenull=false` to exclude null values:

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
package org.opensearch.sql.calcite.remote;

import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_BANK_WITH_NULL_VALUES;
import static org.opensearch.sql.util.MatcherUtils.rows;
import static org.opensearch.sql.util.MatcherUtils.schema;
import static org.opensearch.sql.util.MatcherUtils.verifyDataRows;
import static org.opensearch.sql.util.MatcherUtils.verifyNumOfRows;
import static org.opensearch.sql.util.MatcherUtils.verifySchemaInOrder;

Expand Down Expand Up @@ -41,6 +43,43 @@ public void testTopCommandUseNullFalse() throws IOException {
verifyNumOfRows(result, 5);
}

@Test
public void testTopCommandShowPerc() throws IOException {
JSONObject result =
executeQuery(
String.format("source=%s | top showperc=true age", TEST_INDEX_BANK_WITH_NULL_VALUES));
verifySchemaInOrder(
result, schema("age", "int"), schema("count", "bigint"), schema("percent", "double"));
Comment thread
AjimelecGonzalez marked this conversation as resolved.
verifyNumOfRows(result, 6);
verifyDataRows(
result,
rows(36, 2, 28.571429),
rows(28, 1, 14.285714),
rows(32, 1, 14.285714),
rows(33, 1, 14.285714),
rows(34, 1, 14.285714),
rows(null, 1, 14.285714));
}

@Test
public void testTopCommandShowPercWithoutShowCount() throws IOException {
JSONObject result =
executeQuery(
String.format(
"source=%s | top showperc=true showcount=false age",
TEST_INDEX_BANK_WITH_NULL_VALUES));
verifySchemaInOrder(result, schema("age", "int"), schema("percent", "double"));
Comment thread
AjimelecGonzalez marked this conversation as resolved.
verifyNumOfRows(result, 6);
verifyDataRows(
result,
rows(36, 28.571429),
rows(28, 14.285714),
rows(32, 14.285714),
rows(33, 14.285714),
rows(34, 14.285714),
rows(null, 14.285714));
}

@Test
public void testTopCommandLegacyFalse() throws IOException {
withSettings(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,20 @@ public void testTopNWithGroup() throws IOException {
verifyDataRows(result, rows("F", "TX"), rows("M", "MD"));
}
}

@Test
public void testTopWithShowPerc() throws IOException {
JSONObject result =
executeQuery(String.format("source=%s | top showperc=true gender", TEST_INDEX_ACCOUNT));
if (isCalciteEnabled()) {
verifySchemaInOrder(
result,
schema("gender", "string"),
schema("count", "bigint"),
schema("percent", "double"));
Comment thread
AjimelecGonzalez marked this conversation as resolved.
verifyDataRows(result, rows("M", 507, 50.7), rows("F", 493, 49.3));
} else {
verifyDataRows(result, rows("M"), rows("F"));
}
}
}
2 changes: 2 additions & 0 deletions ppl/src/main/antlr/OpenSearchPPLLexer.g4
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,8 @@ UNION: 'UNION';
MAXOUT: 'MAXOUT';
COUNTFIELD: 'COUNTFIELD';
SHOWCOUNT: 'SHOWCOUNT';
PERCENTFIELD: 'PERCENTFIELD';
SHOWPERC: 'SHOWPERC';
LIMIT: 'LIMIT';
USEOTHER: 'USEOTHER';
OTHERSTR: 'OTHERSTR';
Expand Down
4 changes: 4 additions & 0 deletions ppl/src/main/antlr/OpenSearchPPLParser.g4
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,8 @@ rareTopCommand
rareTopOption
: COUNTFIELD EQUAL countField = stringLiteral
| SHOWCOUNT EQUAL showCount = booleanLiteral
| PERCENTFIELD EQUAL percentField = stringLiteral
| SHOWPERC EQUAL showPerc = booleanLiteral
| USENULL EQUAL useNull = booleanLiteral
;

Expand Down Expand Up @@ -1806,6 +1808,8 @@ searchableKeyWord
| ANOMALY_SCORE_THRESHOLD
| COUNTFIELD
| SHOWCOUNT
| PERCENTFIELD
| SHOWPERC
| MAXOUT
| PATH
| INPUT
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,18 @@ public static List<Argument> getArgumentList(
new Argument(
RareTopN.Option.showCount.name(),
opt.isPresent() ? getArgumentValue(opt.get().showCount) : Literal.TRUE));
opt = ctx.rareTopOption().stream().filter(op -> op.percentField != null).findFirst();
list.add(
new Argument(
RareTopN.Option.percentField.name(),
opt.isPresent()
? getArgumentValue(opt.get().percentField)
: new Literal("percent", DataType.STRING)));
opt = ctx.rareTopOption().stream().filter(op -> op.showPerc != null).findFirst();
list.add(
new Argument(
RareTopN.Option.showPerc.name(),
opt.isPresent() ? getArgumentValue(opt.get().showPerc) : Literal.FALSE));
opt = ctx.rareTopOption().stream().filter(op -> op.useNull != null).findFirst();
list.add(
new Argument(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -460,13 +460,16 @@ public String visitRareTopN(RareTopN node, String context) {
Integer noOfResults = node.getNoOfResults();
String countField = (String) arguments.get(RareTopN.Option.countField.name()).getValue();
Boolean showCount = (Boolean) arguments.get(RareTopN.Option.showCount.name()).getValue();
String percentField = (String) arguments.get(RareTopN.Option.percentField.name()).getValue();
Boolean showPerc = (Boolean) arguments.get(RareTopN.Option.showPerc.name()).getValue();
Boolean useNull = (Boolean) arguments.get(RareTopN.Option.useNull.name()).getValue();
String fields = visitFieldList(node.getFields());
String group = visitExpressionList(node.getGroupExprList());
String options =
UnresolvedPlanHelper.isCalciteEnabled(settings)
? StringUtils.format(
"countield='%s' showcount=%s usenull=%s ", countField, showCount, useNull)
"countfield='%s' showcount=%s percentfield='%s' showperc=%s usenull=%s ",
countField, showCount, percentField, showPerc, useNull)
: "";
return StringUtils.format(
"%s | %s %d %s%s",
Expand Down
Loading
Loading