Skip to content

[feature](nereids)support subquery in select list #23271

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 4 commits into from
Aug 31, 2023
Merged

[feature](nereids)support subquery in select list #23271

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
63e742c
[feature](nereids)support subquery in select list
starocean999 Aug 21, 2023
ae81dc3
fix failed test cases
starocean999 Aug 24, 2023
ee80123
fix fe ut
starocean999 Aug 24, 2023
cd18d91
modified based on comments
starocean999 Aug 25, 2023
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
11 changes: 9 additions & 2 deletions fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/executor/Analyzer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,9 @@
import org.apache.doris.nereids.rules.analysis.CheckAnalysis;
import org.apache.doris.nereids.rules.analysis.CheckBound;
import org.apache.doris.nereids.rules.analysis.CheckPolicy;
import org.apache.doris.nereids.rules.analysis.EliminateGroupByConstant;
import org.apache.doris.nereids.rules.analysis.FillUpMissingSlots;
import org.apache.doris.nereids.rules.analysis.NormalizeAggregate;
import org.apache.doris.nereids.rules.analysis.NormalizeRepeat;
import org.apache.doris.nereids.rules.analysis.ProjectToGlobalAggregate;
import org.apache.doris.nereids.rules.analysis.ProjectWithDistinctToAggregate;
Expand Down Expand Up @@ -110,9 +112,14 @@ private static List<RewriteJob> buildAnalyzeJobs(Optional<CustomTableResolver> c
// LogicalProject for normalize. This rule depends on FillUpMissingSlots to fill up slots.
new NormalizeRepeat()
),
bottomUp(new SubqueryToApply()),
bottomUp(new AdjustAggregateNullableForEmptySet()),
bottomUp(new CheckAnalysis())
// run CheckAnalysis before EliminateGroupByConstant in order to report error message correctly like bellow
// select SUM(lo_tax) FROM lineorder group by 1;
// errCode = 2, detailMessage = GROUP BY expression must not contain aggregate functions: sum(lo_tax)
bottomUp(new CheckAnalysis()),
topDown(new EliminateGroupByConstant()),
topDown(new NormalizeAggregate()),
bottomUp(new SubqueryToApply())
);
}
}
9 changes: 7 additions & 2 deletions fe/fe-core/src/main/java/org/apache/doris/nereids/jobs/executor/Rewriter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@
import org.apache.doris.nereids.rules.analysis.AdjustAggregateNullableForEmptySet;
import org.apache.doris.nereids.rules.analysis.AvgDistinctToSumDivCount;
import org.apache.doris.nereids.rules.analysis.CheckAfterRewrite;
import org.apache.doris.nereids.rules.analysis.EliminateGroupByConstant;
import org.apache.doris.nereids.rules.analysis.LogicalSubQueryAliasToLogicalProject;
import org.apache.doris.nereids.rules.analysis.NormalizeAggregate;
import org.apache.doris.nereids.rules.expression.CheckLegalityAfterRewrite;
import org.apache.doris.nereids.rules.expression.ExpressionNormalization;
import org.apache.doris.nereids.rules.expression.ExpressionOptimization;
Expand All @@ -52,7 +54,6 @@
import org.apache.doris.nereids.rules.rewrite.EliminateDedupJoinCondition;
import org.apache.doris.nereids.rules.rewrite.EliminateEmptyRelation;
import org.apache.doris.nereids.rules.rewrite.EliminateFilter;
import org.apache.doris.nereids.rules.rewrite.EliminateGroupByConstant;
import org.apache.doris.nereids.rules.rewrite.EliminateLimit;
import org.apache.doris.nereids.rules.rewrite.EliminateNotNull;
import org.apache.doris.nereids.rules.rewrite.EliminateNullAwareLeftAntiJoin;
Expand All @@ -74,12 +75,12 @@
import org.apache.doris.nereids.rules.rewrite.MergeOneRowRelationIntoUnion;
import org.apache.doris.nereids.rules.rewrite.MergeProjects;
import org.apache.doris.nereids.rules.rewrite.MergeSetOperations;
import org.apache.doris.nereids.rules.rewrite.NormalizeAggregate;
import org.apache.doris.nereids.rules.rewrite.NormalizeSort;
import org.apache.doris.nereids.rules.rewrite.PruneFileScanPartition;
import org.apache.doris.nereids.rules.rewrite.PruneOlapScanPartition;
import org.apache.doris.nereids.rules.rewrite.PruneOlapScanTablet;
import org.apache.doris.nereids.rules.rewrite.PullUpCteAnchor;
import org.apache.doris.nereids.rules.rewrite.PullUpProjectUnderApply;
import org.apache.doris.nereids.rules.rewrite.PushConjunctsIntoEsScan;
import org.apache.doris.nereids.rules.rewrite.PushConjunctsIntoJdbcScan;
import org.apache.doris.nereids.rules.rewrite.PushFilterInsideJoin;
Expand Down Expand Up @@ -139,6 +140,10 @@ public class Rewriter extends AbstractBatchJobExecutor {
),
// subquery unnesting relay on ExpressionNormalization to extract common factor expression
topic("Subquery unnesting",
// after doing NormalizeAggregate in analysis job
// we need run the following 2 rules to make AGG_SCALAR_SUBQUERY_TO_WINDOW_FUNCTION work
bottomUp(new PullUpProjectUnderApply()),
topDown(new PushdownFilterThroughProject()),
Comment on lines +145 to +146
Copy link
Contributor

@morrySnow morrySnow Aug 25, 2023

Choose a reason for hiding this comment

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

could we put them into costBased(...) ?

costBased(
custom(RuleType.AGG_SCALAR_SUBQUERY_TO_WINDOW_FUNCTION,
AggScalarSubQueryToWindowFunction::new)
Expand Down
3 changes: 2 additions & 1 deletion ...les/rewrite/EliminateGroupByConstant.java → ...es/analysis/EliminateGroupByConstant.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,13 @@
// specific language governing permissions and limitations
// under the License.

package org.apache.doris.nereids.rules.rewrite;
package org.apache.doris.nereids.rules.analysis;

import org.apache.doris.nereids.rules.Rule;
import org.apache.doris.nereids.rules.RuleType;
import org.apache.doris.nereids.rules.expression.ExpressionRewriteContext;
import org.apache.doris.nereids.rules.expression.rules.FoldConstantRule;
import org.apache.doris.nereids.rules.rewrite.OneRewriteRuleFactory;
import org.apache.doris.nereids.trees.expressions.Expression;
import org.apache.doris.nereids.trees.expressions.NamedExpression;
import org.apache.doris.nereids.trees.plans.Plan;
Expand Down
4 changes: 3 additions & 1 deletion ...ids/rules/rewrite/NormalizeAggregate.java → ...ds/rules/analysis/NormalizeAggregate.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,12 @@
// specific language governing permissions and limitations
// under the License.

package org.apache.doris.nereids.rules.rewrite;
package org.apache.doris.nereids.rules.analysis;

import org.apache.doris.nereids.rules.Rule;
import org.apache.doris.nereids.rules.RuleType;
import org.apache.doris.nereids.rules.rewrite.NormalizeToSlot;
import org.apache.doris.nereids.rules.rewrite.OneRewriteRuleFactory;
import org.apache.doris.nereids.trees.expressions.Alias;
import org.apache.doris.nereids.trees.expressions.Expression;
import org.apache.doris.nereids.trees.expressions.NamedExpression;
Expand Down
92 changes: 49 additions & 43 deletions fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/SubqueryToApply.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@
import org.apache.doris.nereids.StatementContext;
import org.apache.doris.nereids.rules.Rule;
import org.apache.doris.nereids.rules.RuleType;
import org.apache.doris.nereids.trees.expressions.Alias;
import org.apache.doris.nereids.trees.expressions.BinaryOperator;
import org.apache.doris.nereids.trees.expressions.CaseWhen;
import org.apache.doris.nereids.trees.expressions.Exists;
import org.apache.doris.nereids.trees.expressions.Expression;
import org.apache.doris.nereids.trees.expressions.InSubquery;
Expand All @@ -47,7 +45,6 @@

import java.util.Collection;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Objects;
Expand All @@ -68,8 +65,8 @@ public List<Rule> buildRules() {
logicalFilter().thenApply(ctx -> {
LogicalFilter<Plan> filter = ctx.root;

ImmutableList<Set> subqueryExprsList = filter.getConjuncts().stream()
.map(e -> (Set) e.collect(SubqueryExpr.class::isInstance))
ImmutableList<Set<SubqueryExpr>> subqueryExprsList = filter.getConjuncts().stream()
.map(e -> (Set<SubqueryExpr>) e.collect(SubqueryExpr.class::isInstance))
.collect(ImmutableList.toImmutableList());
if (subqueryExprsList.stream()
.flatMap(Collection::stream).noneMatch(SubqueryExpr.class::isInstance)) {
Expand Down Expand Up @@ -104,8 +101,7 @@ public List<Rule> buildRules() {
tmpPlan = applyPlan;
newConjuncts.add(conjunct);
}
Set<Expression> conjuncts = new LinkedHashSet<>();
conjuncts.addAll(newConjuncts.build());
Set<Expression> conjuncts = ImmutableSet.copyOf(newConjuncts.build());
Plan newFilter = new LogicalFilter<>(conjuncts, applyPlan);
if (conjuncts.stream().flatMap(c -> c.children().stream())
.anyMatch(MarkJoinSlotReference.class::isInstance)) {
Expand All @@ -116,36 +112,44 @@ public List<Rule> buildRules() {
return new LogicalFilter<>(conjuncts, applyPlan);
})
),
RuleType.PROJECT_SUBQUERY_TO_APPLY.build(
logicalProject().thenApply(ctx -> {
LogicalProject<Plan> project = ctx.root;
Set<SubqueryExpr> subqueryExprs = new LinkedHashSet<>();
project.getProjects().stream()
.filter(Alias.class::isInstance)
.map(Alias.class::cast)
.filter(alias -> alias.child() instanceof CaseWhen)
.forEach(alias -> alias.child().children().stream()
.forEach(e ->
subqueryExprs.addAll(e.collect(SubqueryExpr.class::isInstance))));
if (subqueryExprs.isEmpty()) {
return project;
}

SubqueryContext context = new SubqueryContext(subqueryExprs);
return new LogicalProject(project.getProjects().stream()
.map(p -> p.withChildren(
new ReplaceSubquery(ctx.statementContext, true)
.replace(p, context)))
.collect(ImmutableList.toImmutableList()),
subqueryToApply(
subqueryExprs.stream().collect(ImmutableList.toImmutableList()),
(LogicalPlan) project.child(),
context.getSubqueryToMarkJoinSlot(),
ctx.cascadesContext,
Optional.empty(), true
));
})
)
RuleType.PROJECT_SUBQUERY_TO_APPLY.build(logicalProject().thenApply(ctx -> {
LogicalProject<Plan> project = ctx.root;
ImmutableList<Set<SubqueryExpr>> subqueryExprsList = project.getProjects().stream()
.map(e -> (Set<SubqueryExpr>) e.collect(SubqueryExpr.class::isInstance))
.collect(ImmutableList.toImmutableList());
if (subqueryExprsList.stream().flatMap(Collection::stream).count() == 0) {
return project;
}
List<NamedExpression> oldProjects = ImmutableList.copyOf(project.getProjects());
Copy link
Contributor

@keanji-x keanji-x Aug 30, 2023

Choose a reason for hiding this comment

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

This copy is redundant?

ImmutableList.Builder<NamedExpression> newProjects = new ImmutableList.Builder<>();
LogicalPlan childPlan = (LogicalPlan) project.child();
LogicalPlan applyPlan;
for (int i = 0; i < subqueryExprsList.size(); ++i) {
Set<SubqueryExpr> subqueryExprs = subqueryExprsList.get(i);
if (subqueryExprs.isEmpty()) {
newProjects.add(oldProjects.get(i));
continue;
}

// first step: Replace the subquery in logcialProject's project list
// second step: Replace subquery with LogicalApply
ReplaceSubquery replaceSubquery =
new ReplaceSubquery(ctx.statementContext, true);
SubqueryContext context = new SubqueryContext(subqueryExprs);
Expression newProject =
replaceSubquery.replace(oldProjects.get(i), context);

applyPlan = subqueryToApply(
subqueryExprs.stream().collect(ImmutableList.toImmutableList()),
childPlan, context.getSubqueryToMarkJoinSlot(),
ctx.cascadesContext,
Optional.of(newProject), true);
childPlan = applyPlan;
newProjects.add((NamedExpression) newProject);
}

return project.withProjectsAndChild(newProjects.build(), childPlan);
}))
);
}

Expand Down Expand Up @@ -249,28 +253,30 @@ public Expression visitExistsSubquery(Exists exists, SubqueryContext context) {
// The result set when NULL is specified in the subquery and still evaluates to TRUE by using EXISTS
// When the number of rows returned is empty, agg will return null, so if there is more agg,
// it will always consider the returned result to be true
boolean needCreateMarkJoinSlot = isMarkJoin || isProject;
MarkJoinSlotReference markJoinSlotReference = null;
if (exists.getQueryPlan().anyMatch(Aggregate.class::isInstance) && isMarkJoin) {
if (exists.getQueryPlan().anyMatch(Aggregate.class::isInstance) && needCreateMarkJoinSlot) {
markJoinSlotReference =
new MarkJoinSlotReference(statementContext.generateColumnName(), true);
} else if (isMarkJoin) {
} else if (needCreateMarkJoinSlot) {
markJoinSlotReference =
new MarkJoinSlotReference(statementContext.generateColumnName());
}
if (isMarkJoin) {
if (needCreateMarkJoinSlot) {
context.setSubqueryToMarkJoinSlot(exists, Optional.of(markJoinSlotReference));
}
return isMarkJoin ? markJoinSlotReference : BooleanLiteral.TRUE;
return needCreateMarkJoinSlot ? markJoinSlotReference : BooleanLiteral.TRUE;
}

@Override
public Expression visitInSubquery(InSubquery in, SubqueryContext context) {
MarkJoinSlotReference markJoinSlotReference =
new MarkJoinSlotReference(statementContext.generateColumnName());
if (isMarkJoin) {
boolean needCreateMarkJoinSlot = isMarkJoin || isProject;
if (needCreateMarkJoinSlot) {
context.setSubqueryToMarkJoinSlot(in, Optional.of(markJoinSlotReference));
}
return isMarkJoin ? markJoinSlotReference : BooleanLiteral.TRUE;
return needCreateMarkJoinSlot ? markJoinSlotReference : BooleanLiteral.TRUE;
}

@Override
Expand Down
2 changes: 1 addition & 1 deletion ...core/src/main/java/org/apache/doris/nereids/rules/implementation/AggregateStrategies.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@
import org.apache.doris.nereids.properties.RequireProperties;
import org.apache.doris.nereids.rules.Rule;
import org.apache.doris.nereids.rules.RuleType;
import org.apache.doris.nereids.rules.analysis.NormalizeAggregate;
import org.apache.doris.nereids.rules.expression.rules.FoldConstantRuleOnFE;
import org.apache.doris.nereids.rules.rewrite.NormalizeAggregate;
import org.apache.doris.nereids.trees.expressions.AggregateExpression;
import org.apache.doris.nereids.trees.expressions.Alias;
import org.apache.doris.nereids.trees.expressions.Cast;
Expand Down
5 changes: 4 additions & 1 deletion ...e-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PushdownAliasThroughJoin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.apache.doris.nereids.trees.expressions.Alias;
import org.apache.doris.nereids.trees.expressions.ExprId;
import org.apache.doris.nereids.trees.expressions.Expression;
import org.apache.doris.nereids.trees.expressions.MarkJoinSlotReference;
import org.apache.doris.nereids.trees.expressions.NamedExpression;
import org.apache.doris.nereids.trees.expressions.Slot;
import org.apache.doris.nereids.trees.plans.Plan;
Expand All @@ -45,7 +46,9 @@ public class PushdownAliasThroughJoin extends OneRewriteRuleFactory {
public Rule build() {
return logicalProject(logicalJoin())
.when(project -> project.getProjects().stream().allMatch(expr ->
(expr instanceof Slot) || (expr instanceof Alias && ((Alias) expr).child() instanceof Slot)))
(expr instanceof Slot && !(expr instanceof MarkJoinSlotReference))
|| (expr instanceof Alias && ((Alias) expr).child() instanceof Slot
&& !(((Alias) expr).child() instanceof MarkJoinSlotReference))))
Comment on lines +49 to +51
Copy link
Contributor

@morrySnow morrySnow Aug 25, 2023

Choose a reason for hiding this comment

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

do we need add more ut about this change?

.when(project -> project.getProjects().stream().anyMatch(expr -> expr instanceof Alias))
.then(project -> {
LogicalJoin<? extends Plan, ? extends Plan> join = project.child();
Expand Down
2 changes: 1 addition & 1 deletion fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/CaseWhen.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public String toString() {
StringBuilder output = new StringBuilder("CASE");
for (Expression child : children()) {
if (child instanceof WhenClause) {
output.append(child);
output.append(child.toString());
} else {
output.append(" ELSE ").append(child.toString());
}
Expand Down
5 changes: 5 additions & 0 deletions ...-core/src/main/java/org/apache/doris/nereids/trees/expressions/literal/DoubleLiteral.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,4 +58,9 @@ public String toString() {
nf.setGroupingUsed(false);
return nf.format(value);
}

@Override
public String getStringValue() {
return toString();
}
}
9 changes: 9 additions & 0 deletions ...e-core/src/main/java/org/apache/doris/nereids/trees/expressions/literal/FloatLiteral.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor;
import org.apache.doris.nereids.types.FloatType;

import java.text.NumberFormat;

/**
* float type literal
*/
Expand All @@ -48,4 +50,11 @@ public <R, C> R accept(ExpressionVisitor<R, C> visitor, C context) {
public LiteralExpr toLegacyLiteral() {
return new org.apache.doris.analysis.FloatLiteral((double) value, Type.FLOAT);
}

@Override
public String getStringValue() {
NumberFormat nf = NumberFormat.getInstance();
nf.setGroupingUsed(false);
return nf.format(value);
}
}
2 changes: 1 addition & 1 deletion fe/fe-core/src/test/java/org/apache/doris/nereids/rules/analysis/AnalyzeCTETest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ public void testCTEInHavingAndSubquery() {
logicalFilter(
logicalProject(
logicalJoin(
logicalAggregate(),
logicalProject(logicalAggregate()),
logicalProject()
)
)
Expand Down
Loading