morrySnow commented on code in PR #40048:
URL: https://github.com/apache/doris/pull/40048#discussion_r1738100475
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java:
##########
@@ -1385,6 +1390,66 @@ public LogicalPlan
visitRegularQuerySpecification(RegularQuerySpecificationConte
});
}
+ private LogicalPlan withQualifyQuery(LogicalPlan input,
RegularQuerySpecificationContext ctx) {
+ QualifyClauseContext qualifyClauseContext = ctx.qualifyClause();
+ if (qualifyClauseContext != null) {
+ NamedExpression qualifyExpr =
visitNamedExpression(qualifyClauseContext.namedExpression());
+ Expression filter;
+ List<NamedExpression> excepts = new ArrayList<>();
+ if (qualifyExpr instanceof UnboundAlias) {
+ Expression cmp = qualifyExpr.child(0);
+ if (!(cmp instanceof ComparisonPredicate)) {
+ throw new ParseException("only support compare expr");
+ }
+ cmp = ExpressionRuleExecutor.normalize(cmp);
+ Expression left = cmp.child(0);
+ if (left instanceof WindowExpression) {
+ WindowExpression windowExpression = (WindowExpression)
left;
+ String funName =
windowExpression.getFunction().getExpressionName();
+ switch (funName.toLowerCase()) {
+ case "row_number":
+ case "rank":
+ case "dense_rank":
+ break;
+ default:
+ throw new ParseException("not support window
function : " + funName);
+ }
+ excepts.add(new UnboundSlot("_QUALIFY_COLUMN"));
+ }
+ if (left instanceof UnboundSlot) {
+ left = new UnboundSlot(((UnboundSlot)
left).getNameParts());
+ } else {
+ left = new UnboundSlot("_QUALIFY_COLUMN");
+ }
+ Expression right = cmp.child(1);
+ if (!(right instanceof IntegerLikeLiteral)) {
+ throw new ParseException("qualify expression only support
constant compare");
+ }
+ if (cmp instanceof EqualTo) {
+ filter = new EqualTo(left, right);
+ } else if (cmp instanceof GreaterThan) {
+ filter = new GreaterThan(left, right);
+ } else if (cmp instanceof GreaterThanEqual) {
+ filter = new GreaterThanEqual(left, right);
+ } else if (cmp instanceof LessThan) {
+ filter = new LessThan(left, right);
+ } else if (cmp instanceof LessThanEqual) {
+ filter = new LessThanEqual(left, right);
+ } else {
+ throw new ParseException("not support compare type " +
cmp.getClass().getSimpleName());
+ }
+ } else {
+ throw new ParseException("not support column type");
+ }
+ LogicalFilter<LogicalPlan> logicalFilter = new
LogicalFilter<>(Sets.newHashSet(filter), input);
+ List<NamedExpression> output =
+ Lists.newArrayList(new UnboundStar(ImmutableList.of(),
excepts, ImmutableList.of()));
+ LogicalPlan plan = withQueryOrganization(logicalFilter,
ctx.queryOrganization());
+ return new LogicalProject<>(output, plan);
+ }
+ return withQueryOrganization(input, ctx.queryOrganization());
Review Comment:
do not put withQueryOrganization into withQualify
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java:
##########
@@ -1370,8 +1374,9 @@ public LogicalPlan
visitRegularQuerySpecification(RegularQuerySpecificationConte
selectCtx,
Optional.ofNullable(ctx.whereClause()),
Optional.ofNullable(ctx.aggClause()),
- Optional.ofNullable(ctx.havingClause()));
- selectPlan = withQueryOrganization(selectPlan,
ctx.queryOrganization());
+ Optional.ofNullable(ctx.havingClause()),
+ Optional.ofNullable(ctx.qualifyClause()));
+ selectPlan = withQualifyQuery(selectPlan, ctx);
Review Comment:
why pass `qualifyClause` to `withSelectQuerySpecification` but not process
it?
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java:
##########
@@ -1385,6 +1390,66 @@ public LogicalPlan
visitRegularQuerySpecification(RegularQuerySpecificationConte
});
}
+ private LogicalPlan withQualifyQuery(LogicalPlan input,
RegularQuerySpecificationContext ctx) {
+ QualifyClauseContext qualifyClauseContext = ctx.qualifyClause();
+ if (qualifyClauseContext != null) {
+ NamedExpression qualifyExpr =
visitNamedExpression(qualifyClauseContext.namedExpression());
+ Expression filter;
+ List<NamedExpression> excepts = new ArrayList<>();
+ if (qualifyExpr instanceof UnboundAlias) {
+ Expression cmp = qualifyExpr.child(0);
+ if (!(cmp instanceof ComparisonPredicate)) {
+ throw new ParseException("only support compare expr");
+ }
+ cmp = ExpressionRuleExecutor.normalize(cmp);
+ Expression left = cmp.child(0);
+ if (left instanceof WindowExpression) {
+ WindowExpression windowExpression = (WindowExpression)
left;
+ String funName =
windowExpression.getFunction().getExpressionName();
+ switch (funName.toLowerCase()) {
+ case "row_number":
+ case "rank":
+ case "dense_rank":
+ break;
+ default:
+ throw new ParseException("not support window
function : " + funName);
+ }
+ excepts.add(new UnboundSlot("_QUALIFY_COLUMN"));
+ }
+ if (left instanceof UnboundSlot) {
+ left = new UnboundSlot(((UnboundSlot)
left).getNameParts());
+ } else {
+ left = new UnboundSlot("_QUALIFY_COLUMN");
+ }
+ Expression right = cmp.child(1);
+ if (!(right instanceof IntegerLikeLiteral)) {
+ throw new ParseException("qualify expression only support
constant compare");
+ }
Review Comment:
why must integer literal? how about `rn > col_x`
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java:
##########
@@ -1385,6 +1390,66 @@ public LogicalPlan
visitRegularQuerySpecification(RegularQuerySpecificationConte
});
}
+ private LogicalPlan withQualifyQuery(LogicalPlan input,
RegularQuerySpecificationContext ctx) {
+ QualifyClauseContext qualifyClauseContext = ctx.qualifyClause();
+ if (qualifyClauseContext != null) {
+ NamedExpression qualifyExpr =
visitNamedExpression(qualifyClauseContext.namedExpression());
+ Expression filter;
+ List<NamedExpression> excepts = new ArrayList<>();
+ if (qualifyExpr instanceof UnboundAlias) {
+ Expression cmp = qualifyExpr.child(0);
+ if (!(cmp instanceof ComparisonPredicate)) {
+ throw new ParseException("only support compare expr");
+ }
+ cmp = ExpressionRuleExecutor.normalize(cmp);
+ Expression left = cmp.child(0);
+ if (left instanceof WindowExpression) {
+ WindowExpression windowExpression = (WindowExpression)
left;
+ String funName =
windowExpression.getFunction().getExpressionName();
+ switch (funName.toLowerCase()) {
+ case "row_number":
+ case "rank":
+ case "dense_rank":
+ break;
+ default:
+ throw new ParseException("not support window
function : " + funName);
+ }
+ excepts.add(new UnboundSlot("_QUALIFY_COLUMN"));
+ }
+ if (left instanceof UnboundSlot) {
+ left = new UnboundSlot(((UnboundSlot)
left).getNameParts());
+ } else {
+ left = new UnboundSlot("_QUALIFY_COLUMN");
+ }
+ Expression right = cmp.child(1);
+ if (!(right instanceof IntegerLikeLiteral)) {
+ throw new ParseException("qualify expression only support
constant compare");
+ }
+ if (cmp instanceof EqualTo) {
+ filter = new EqualTo(left, right);
+ } else if (cmp instanceof GreaterThan) {
+ filter = new GreaterThan(left, right);
+ } else if (cmp instanceof GreaterThanEqual) {
+ filter = new GreaterThanEqual(left, right);
+ } else if (cmp instanceof LessThan) {
+ filter = new LessThan(left, right);
+ } else if (cmp instanceof LessThanEqual) {
+ filter = new LessThanEqual(left, right);
Review Comment:
we should support all expression here
##########
fe/fe-core/src/main/antlr4/org/apache/doris/nereids/DorisParser.g4:
##########
@@ -804,6 +805,10 @@ havingClause
: HAVING booleanExpression
;
+qualifyClause
+ : QUALIFY namedExpression
Review Comment:
why qualify follow with `namedExpression ` not `booleanExpression`?
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java:
##########
@@ -1385,6 +1390,66 @@ public LogicalPlan
visitRegularQuerySpecification(RegularQuerySpecificationConte
});
}
+ private LogicalPlan withQualifyQuery(LogicalPlan input,
RegularQuerySpecificationContext ctx) {
+ QualifyClauseContext qualifyClauseContext = ctx.qualifyClause();
+ if (qualifyClauseContext != null) {
+ NamedExpression qualifyExpr =
visitNamedExpression(qualifyClauseContext.namedExpression());
+ Expression filter;
+ List<NamedExpression> excepts = new ArrayList<>();
+ if (qualifyExpr instanceof UnboundAlias) {
+ Expression cmp = qualifyExpr.child(0);
+ if (!(cmp instanceof ComparisonPredicate)) {
+ throw new ParseException("only support compare expr");
+ }
+ cmp = ExpressionRuleExecutor.normalize(cmp);
+ Expression left = cmp.child(0);
+ if (left instanceof WindowExpression) {
+ WindowExpression windowExpression = (WindowExpression)
left;
+ String funName =
windowExpression.getFunction().getExpressionName();
+ switch (funName.toLowerCase()) {
+ case "row_number":
+ case "rank":
+ case "dense_rank":
+ break;
+ default:
+ throw new ParseException("not support window
function : " + funName);
+ }
+ excepts.add(new UnboundSlot("_QUALIFY_COLUMN"));
Review Comment:
why use magic string literal `_QUALIFY_COLUMN `
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java:
##########
@@ -1385,6 +1390,66 @@ public LogicalPlan
visitRegularQuerySpecification(RegularQuerySpecificationConte
});
}
+ private LogicalPlan withQualifyQuery(LogicalPlan input,
RegularQuerySpecificationContext ctx) {
+ QualifyClauseContext qualifyClauseContext = ctx.qualifyClause();
+ if (qualifyClauseContext != null) {
+ NamedExpression qualifyExpr =
visitNamedExpression(qualifyClauseContext.namedExpression());
+ Expression filter;
+ List<NamedExpression> excepts = new ArrayList<>();
+ if (qualifyExpr instanceof UnboundAlias) {
+ Expression cmp = qualifyExpr.child(0);
+ if (!(cmp instanceof ComparisonPredicate)) {
+ throw new ParseException("only support compare expr");
+ }
+ cmp = ExpressionRuleExecutor.normalize(cmp);
+ Expression left = cmp.child(0);
+ if (left instanceof WindowExpression) {
+ WindowExpression windowExpression = (WindowExpression)
left;
+ String funName =
windowExpression.getFunction().getExpressionName();
+ switch (funName.toLowerCase()) {
+ case "row_number":
+ case "rank":
+ case "dense_rank":
+ break;
+ default:
+ throw new ParseException("not support window
function : " + funName);
+ }
+ excepts.add(new UnboundSlot("_QUALIFY_COLUMN"));
+ }
+ if (left instanceof UnboundSlot) {
+ left = new UnboundSlot(((UnboundSlot)
left).getNameParts());
+ } else {
+ left = new UnboundSlot("_QUALIFY_COLUMN");
+ }
+ Expression right = cmp.child(1);
+ if (!(right instanceof IntegerLikeLiteral)) {
+ throw new ParseException("qualify expression only support
constant compare");
+ }
+ if (cmp instanceof EqualTo) {
+ filter = new EqualTo(left, right);
+ } else if (cmp instanceof GreaterThan) {
+ filter = new GreaterThan(left, right);
+ } else if (cmp instanceof GreaterThanEqual) {
+ filter = new GreaterThanEqual(left, right);
+ } else if (cmp instanceof LessThan) {
+ filter = new LessThan(left, right);
+ } else if (cmp instanceof LessThanEqual) {
+ filter = new LessThanEqual(left, right);
+ } else {
+ throw new ParseException("not support compare type " +
cmp.getClass().getSimpleName());
+ }
+ } else {
+ throw new ParseException("not support column type");
+ }
+ LogicalFilter<LogicalPlan> logicalFilter = new
LogicalFilter<>(Sets.newHashSet(filter), input);
Review Comment:
should not convert to filter here, because qualify should only allow when
window function exists in itself or in project list
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java:
##########
@@ -1385,6 +1390,66 @@ public LogicalPlan
visitRegularQuerySpecification(RegularQuerySpecificationConte
});
}
+ private LogicalPlan withQualifyQuery(LogicalPlan input,
RegularQuerySpecificationContext ctx) {
+ QualifyClauseContext qualifyClauseContext = ctx.qualifyClause();
+ if (qualifyClauseContext != null) {
+ NamedExpression qualifyExpr =
visitNamedExpression(qualifyClauseContext.namedExpression());
+ Expression filter;
+ List<NamedExpression> excepts = new ArrayList<>();
+ if (qualifyExpr instanceof UnboundAlias) {
+ Expression cmp = qualifyExpr.child(0);
+ if (!(cmp instanceof ComparisonPredicate)) {
+ throw new ParseException("only support compare expr");
+ }
+ cmp = ExpressionRuleExecutor.normalize(cmp);
+ Expression left = cmp.child(0);
+ if (left instanceof WindowExpression) {
Review Comment:
what will happen if write `rn + col_x > 10`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
