Skip to content

[8.x] Disallow ES|QL CATEGORIZE in aggregation filters (#118319) #118330

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.

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
merged 2 commits into from
Dec 12, 2024
Merged
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 @@ -382,6 +382,18 @@ private static void checkCategorizeGrouping(Aggregate agg, Set<Failure> failures
);
}
})));
agg.aggregates().forEach(a -> a.forEachDown(FilteredExpression.class, fe -> fe.filter().forEachDown(Attribute.class, attribute -> {
var categorize = categorizeByAttribute.get(attribute);
if (categorize != null) {
failures.add(
fail(
attribute,
"cannot reference CATEGORIZE grouping function [{}] within an aggregation filter",
attribute.sourceText()
)
);
}
})));
}

private static void checkRateAggregates(Expression expr, int nestedLevel, Set<Failure> failures) {
Expand Down Expand Up @@ -421,7 +433,8 @@ private static void checkInvalidNamedExpressionUsage(
Expression filter = fe.filter();
failures.add(fail(filter, "WHERE clause allowed only for aggregate functions, none found in [{}]", fe.sourceText()));
}
Expression f = fe.filter(); // check the filter has to be a boolean term, similar as checkFilterConditionType
Expression f = fe.filter();
// check the filter has to be a boolean term, similar as checkFilterConditionType
if (f.dataType() != NULL && f.dataType() != BOOLEAN) {
failures.add(fail(f, "Condition expression needs to be boolean, found [{}]", f.dataType()));
}
Expand All @@ -432,9 +445,10 @@ private static void checkInvalidNamedExpressionUsage(
fail(af, "cannot use aggregate function [{}] in aggregate WHERE clause [{}]", af.sourceText(), fe.sourceText())
);
}
// check the bucketing function against the group
// check the grouping function against the group
else if (c instanceof GroupingFunction gf) {
if (Expressions.anyMatch(groups, ex -> ex instanceof Alias a && a.child().semanticEquals(gf)) == false) {
if (c instanceof Categorize
|| Expressions.anyMatch(groups, ex -> ex instanceof Alias a && a.child().semanticEquals(gf)) == false) {
failures.add(fail(gf, "can only use grouping function [{}] as part of the BY clause", gf.sourceText()));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1968,6 +1968,26 @@ public void testCategorizeWithinAggregations() {
);
}

public void testCategorizeWithFilteredAggregations() {
assumeTrue("requires Categorize capability", EsqlCapabilities.Cap.CATEGORIZE_V5.isEnabled());

query("FROM test | STATS COUNT(*) WHERE first_name == \"John\" BY CATEGORIZE(last_name)");
query("FROM test | STATS COUNT(*) WHERE last_name == \"Doe\" BY CATEGORIZE(last_name)");

assertEquals(
"1:34: can only use grouping function [CATEGORIZE(first_name)] as part of the BY clause",
error("FROM test | STATS COUNT(*) WHERE CATEGORIZE(first_name) == \"John\" BY CATEGORIZE(last_name)")
);
assertEquals(
"1:34: can only use grouping function [CATEGORIZE(last_name)] as part of the BY clause",
error("FROM test | STATS COUNT(*) WHERE CATEGORIZE(last_name) == \"Doe\" BY CATEGORIZE(last_name)")
);
assertEquals(
"1:34: cannot reference CATEGORIZE grouping function [category] within an aggregation filter",
error("FROM test | STATS COUNT(*) WHERE category == \"Doe\" BY category = CATEGORIZE(last_name)")
);
}

public void testSortByAggregate() {
assertEquals("1:18: Aggregate functions are not allowed in SORT [COUNT]", error("ROW a = 1 | SORT count(*)"));
assertEquals("1:28: Aggregate functions are not allowed in SORT [COUNT]", error("ROW a = 1 | SORT to_string(count(*))"));
Expand Down
Loading