Skip to content

[fix](nereids)having with no group by is not parsed correctly #14883

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
morrySnow merged 2 commits into from
Dec 12, 2022
Merged

[fix](nereids)having with no group by is not parsed correctly #14883

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
46c70b2
[fix](having)having with no group by is not parsed correctly
starocean999 Dec 7, 2022
175a853
fix having with no agg node bug
starocean999 Dec 12, 2022
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
26 changes: 22 additions & 4 deletions fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -986,11 +986,29 @@ private LogicalPlan withSelectQuerySpecification(
// from -> where -> group by -> having -> select

LogicalPlan filter = withFilter(inputRelation, whereClause);
SelectColumnClauseContext columnCtx = selectClause.selectColumnClause();
LogicalPlan aggregate = withAggregate(filter, columnCtx, aggClause);
SelectColumnClauseContext selectColumnCtx = selectClause.selectColumnClause();
LogicalPlan aggregate = withAggregate(filter, selectColumnCtx, aggClause);
// TODO: replace and process having at this position
LogicalPlan having = withHaving(aggregate, havingClause);
return withProjection(having, selectClause.selectColumnClause(), aggClause);
if (!(aggregate instanceof Aggregate) && havingClause.isPresent()) {
// create a project node for pattern match of ProjectToGlobalAggregate rule
// then ProjectToGlobalAggregate rule can insert agg node as LogicalHaving node's child
LogicalPlan project;
if (selectColumnCtx.EXCEPT() != null) {
List<NamedExpression> expressions = getNamedExpressions(selectColumnCtx.namedExpressionSeq());
if (!expressions.stream().allMatch(UnboundSlot.class::isInstance)) {
throw new ParseException("only column name is supported in except clause", selectColumnCtx);
}
project = new LogicalProject<>(ImmutableList.of(new UnboundStar(Collections.emptyList())),
expressions, aggregate);
} else {
List<NamedExpression> projects = getNamedExpressions(selectColumnCtx.namedExpressionSeq());
project = new LogicalProject<>(projects, Collections.emptyList(), aggregate);
}
return new LogicalHaving<>(getExpression((havingClause.get().booleanExpression())), project);
} else {
LogicalPlan having = withHaving(aggregate, havingClause);
return withProjection(having, selectColumnCtx, aggClause);
}
});
}

Expand Down
1 change: 1 addition & 0 deletions fe/fe-core/src/main/java/org/apache/doris/nereids/rules/RuleType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ public enum RuleType {
REPLACE_SORT_EXPRESSION_BY_CHILD_OUTPUT(RuleTypeClass.REWRITE),

FILL_UP_HAVING_AGGREGATE(RuleTypeClass.REWRITE),
FILL_UP_HAVING_PROJECT(RuleTypeClass.REWRITE),
FILL_UP_SORT_AGGREGATE(RuleTypeClass.REWRITE),
FILL_UP_SORT_HAVING_AGGREGATE(RuleTypeClass.REWRITE),
FILL_UP_SORT_PROJECT(RuleTypeClass.REWRITE),
Expand Down
8 changes: 4 additions & 4 deletions fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindSlotReference.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,12 +246,12 @@ public List<Rule> buildRules() {
})
),
RuleType.BINDING_HAVING_SLOT.build(
logicalHaving(aggregate()).when(Plan::canBind).thenApply(ctx -> {
LogicalHaving<Aggregate<GroupPlan>> having = ctx.root;
Aggregate<GroupPlan> aggregate = having.child();
logicalHaving(any()).when(Plan::canBind).thenApply(ctx -> {
LogicalHaving<Plan> having = ctx.root;
Plan childPlan = having.child();
// We should deduplicate the slots, otherwise the binding process will fail due to the
// ambiguous slots exist.
Set<Slot> boundSlots = Stream.concat(Stream.of(aggregate), aggregate.children().stream())
Set<Slot> boundSlots = Stream.concat(Stream.of(childPlan), childPlan.children().stream())
.flatMap(plan -> plan.getOutput().stream())
.collect(Collectors.toSet());
Expression boundPredicates = new SlotBinder(
Expand Down
5 changes: 4 additions & 1 deletion fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/FillUpMissingSlots.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,10 @@ public List<Rule> buildRules() {
having.getPredicates(), r.getSubstitution());
return new LogicalFilter<>(newPredicates, a);
});
})
})),
RuleType.FILL_UP_HAVING_PROJECT.build(
Copy link
Contributor

@morrySnow morrySnow Dec 12, 2022

Choose a reason for hiding this comment

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

rule's name is better called HAVING_TO_FILTER

logicalHaving(logicalProject()).then(having -> new LogicalFilter<>(having.getPredicates(),
having.child()))
)
);
}
Expand Down
46 changes: 46 additions & 0 deletions regression-test/data/nereids_syntax_p0/having.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,49 @@
2
3

-- !select --
9

-- !select --
1
1
1
2
2
2
3
3
3

-- !select --
1
1
1
2
2
2
3
3
3

-- !select --
36

-- !select --
36

-- !select --
36

-- !select --
36

-- !select --
54

-- !select --
54

-- !select --
9

12 changes: 12 additions & 0 deletions regression-test/suites/nereids_syntax_p0/having.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
suite("test_nereids_having") {

sql "SET enable_nereids_planner=true"
sql "SET enable_fallback_to_original_planner=true"
sql "SET enable_vectorized_engine=true"

sql "DROP TABLE IF EXISTS test_nereids_having_tbl"
Expand Down Expand Up @@ -57,4 +58,15 @@ suite("test_nereids_having") {
order_qt_select "SELECT a1, SUM(a1 + a2) FROM test_nereids_having_tbl GROUP BY a1 HAVING SUM(a1 + a2) > 0";
order_qt_select "SELECT a1, SUM(a1 + a2) FROM test_nereids_having_tbl GROUP BY a1 HAVING SUM(a1 + a2 + 3) > 0";
order_qt_select "SELECT a1 FROM test_nereids_having_tbl GROUP BY a1 HAVING COUNT(*) > 0";
order_qt_select "SELECT COUNT(*) FROM test_nereids_having_tbl HAVING COUNT(*) > 0";

order_qt_select "SELECT a1 as value FROM test_nereids_having_tbl HAVING a1 > 0";
order_qt_select "SELECT a1 as value FROM test_nereids_having_tbl HAVING value > 0";
order_qt_select "SELECT SUM(a2) FROM test_nereids_having_tbl HAVING SUM(a2) > 0";
order_qt_select "SELECT SUM(a2) as value FROM test_nereids_having_tbl HAVING SUM(a2) > 0";
order_qt_select "SELECT SUM(a2) as value FROM test_nereids_having_tbl HAVING value > 0";
order_qt_select "SELECT SUM(a2) FROM test_nereids_having_tbl HAVING MIN(pk) > 0";
order_qt_select "SELECT SUM(a1 + a2) FROM test_nereids_having_tbl HAVING SUM(a1 + a2) > 0";
order_qt_select "SELECT SUM(a1 + a2) FROM test_nereids_having_tbl HAVING SUM(a1 + a2 + 3) > 0";
order_qt_select "SELECT COUNT(*) FROM test_nereids_having_tbl HAVING COUNT(*) > 0";
}