morrySnow commented on code in PR #10415:
URL: https://github.com/apache/doris/pull/10415#discussion_r906932104
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/analyzer/UnboundAlias.java:
##########
@@ -34,18 +38,28 @@
extends NamedExpression
implements UnaryExpression<CHILD_TYPE>, Unbound {
- public UnboundAlias(CHILD_TYPE child) {
+ private final ExprId exprId;
+ private final String name;
+
+ public UnboundAlias(ExprId exprId, CHILD_TYPE child) {
super(NodeType.UNBOUND_ALIAS, child);
+ this.exprId = Objects.requireNonNull(exprId, "exprId can not be null");
+ this.name = "expr_" + exprId;
}
@Override
public String sql() {
- return null;
+ return child().sql();
Review Comment:
i think all unbound should not generate sql
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java:
##########
@@ -143,265 +130,47 @@ public Object visitChildren(RuleNode node) {
@Override
public LogicalPlan visitSingleStatement(SingleStatementContext ctx) {
- Supplier<LogicalPlan> f = () -> (LogicalPlan) visit(ctx.statement());
- return ParserUtils.withOrigin(ctx, f);
+ return ParserUtils.withOrigin(ctx, () -> (LogicalPlan)
visit(ctx.statement()));
}
/*
********************************************************************************************
* Plan parsing
*
********************************************************************************************
*/
- private LogicalPlan plan(ParserRuleContext tree) {
- return (LogicalPlan) tree.accept(this);
- }
-
@Override
public LogicalPlan visitQuery(QueryContext ctx) {
- Supplier<LogicalPlan> f = () -> {
+ return ParserUtils.withOrigin(ctx, () -> {
// TODO: need to add withQueryResultClauses and withCTE
LogicalPlan query = plan(ctx.queryTerm());
- LogicalPlan queryOrganization =
withQueryOrganization(ctx.queryOrganization(), query);
+ LogicalPlan queryOrganization = withOrganization(query,
ctx.queryOrganization());
return queryOrganization;
- };
- return ParserUtils.withOrigin(ctx, f);
- }
-
- private LogicalPlan withQueryOrganization(QueryOrganizationContext ctx,
LogicalPlan children) {
- List<SortItems> sortItems = visitQueryOrganization(ctx);
- return sortItems == null ? children : new LogicalUnaryPlan(new
LogicalSort(sortItems), children);
+ });
}
@Override
public LogicalPlan
visitRegularQuerySpecification(RegularQuerySpecificationContext ctx) {
- Supplier<LogicalPlan> f = () -> {
+ return ParserUtils.withOrigin(ctx, () -> {
// TODO: support on row relation
- LogicalPlan from = visitFromClause(ctx.fromClause());
+ LogicalPlan relation =
withRelation(Optional.ofNullable(ctx.fromClause()));
return withSelectQuerySpecification(
- ctx,
- ctx.selectClause(),
- ctx.whereClause(),
- from,
- ctx.aggClause());
- };
- return ParserUtils.withOrigin(ctx, f);
- }
-
- @Override
- public Expression visitExpression(ExpressionContext ctx) {
- Supplier<Expression> f = () -> (Expression)
visit(ctx.booleanExpression());
- return ParserUtils.withOrigin(ctx, f);
- }
-
- @Override
- public List<Expression> visitNamedExpressionSeq(NamedExpressionSeqContext
ctx) {
- List<Expression> expressions = Lists.newArrayList();
- if (ctx != null) {
- for (NamedExpressionContext namedExpressionContext :
ctx.namedExpression()) {
- Expression expression = typedVisit(namedExpressionContext);
- expressions.add(expression);
- }
- }
- return expressions;
- }
-
- /**
- * Add a regular (SELECT) query specification to a logical plan. The query
specification
- * is the core of the logical plan, this is where sourcing (FROM clause),
projection (SELECT),
- * aggregation (GROUP BY ... HAVING ...) and filtering (WHERE) takes place.
- *
- * <p>Note that query hints are ignored (both by the parser and the
builder).
- */
- private LogicalPlan withSelectQuerySpecification(
- ParserRuleContext ctx,
- SelectClauseContext selectClause,
- WhereClauseContext whereClause,
- LogicalPlan relation,
- AggClauseContext aggClause) {
- Supplier<LogicalPlan> f = () -> {
- // Filter(expression(ctx.booleanExpression), plan);
- LogicalPlan plan = visitCommonSelectQueryClausePlan(
- relation,
- visitNamedExpressionSeq(selectClause.namedExpressionSeq()),
- whereClause,
- aggClause);
- // TODO: process hint
- return plan;
- };
- return ParserUtils.withOrigin(ctx, f);
- }
-
- private LogicalPlan visitCommonSelectQueryClausePlan(
- LogicalPlan relation,
- List<Expression> expressions,
- WhereClauseContext whereClause,
- AggClauseContext aggClause) {
- // TODO: add lateral views
- // val withLateralView =
lateralView.asScala.foldLeft(relation)(withGenerate)
-
- // add where
- LogicalPlan withFilter = relation.optionalMap(whereClause,
withWhereClause);
-
- List<NamedExpression> namedExpressions =
expressions.stream().map(expression -> {
- if (expression instanceof NamedExpression) {
- return (NamedExpression) expression;
- } else {
- return new UnboundAlias(expression);
- }
- }).collect(Collectors.toList());
-
- LogicalPlan withProject;
- if (CollectionUtils.isNotEmpty(namedExpressions)) {
- withProject = new LogicalUnaryPlan(new
LogicalProject(namedExpressions), withFilter);
- } else {
- withProject = withFilter;
- }
-
- LogicalPlan withAgg;
- if (aggClause != null) {
- withAgg = withAggClause(namedExpressions, aggClause.groupByItem(),
withFilter);
- } else {
- withAgg = withProject;
- }
-
- return withAgg;
- }
-
- @Override
- public LogicalPlan visitFromClause(FromClauseContext ctx) {
Review Comment:
should not remove this function, from clause use in more than one place in
the future
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java:
##########
@@ -143,265 +130,47 @@ public Object visitChildren(RuleNode node) {
@Override
public LogicalPlan visitSingleStatement(SingleStatementContext ctx) {
- Supplier<LogicalPlan> f = () -> (LogicalPlan) visit(ctx.statement());
- return ParserUtils.withOrigin(ctx, f);
+ return ParserUtils.withOrigin(ctx, () -> (LogicalPlan)
visit(ctx.statement()));
}
/*
********************************************************************************************
* Plan parsing
*
********************************************************************************************
*/
- private LogicalPlan plan(ParserRuleContext tree) {
- return (LogicalPlan) tree.accept(this);
- }
-
@Override
public LogicalPlan visitQuery(QueryContext ctx) {
- Supplier<LogicalPlan> f = () -> {
+ return ParserUtils.withOrigin(ctx, () -> {
// TODO: need to add withQueryResultClauses and withCTE
LogicalPlan query = plan(ctx.queryTerm());
- LogicalPlan queryOrganization =
withQueryOrganization(ctx.queryOrganization(), query);
+ LogicalPlan queryOrganization = withOrganization(query,
ctx.queryOrganization());
return queryOrganization;
- };
- return ParserUtils.withOrigin(ctx, f);
- }
-
- private LogicalPlan withQueryOrganization(QueryOrganizationContext ctx,
LogicalPlan children) {
- List<SortItems> sortItems = visitQueryOrganization(ctx);
- return sortItems == null ? children : new LogicalUnaryPlan(new
LogicalSort(sortItems), children);
+ });
}
@Override
public LogicalPlan
visitRegularQuerySpecification(RegularQuerySpecificationContext ctx) {
- Supplier<LogicalPlan> f = () -> {
+ return ParserUtils.withOrigin(ctx, () -> {
// TODO: support on row relation
- LogicalPlan from = visitFromClause(ctx.fromClause());
+ LogicalPlan relation =
withRelation(Optional.ofNullable(ctx.fromClause()));
Review Comment:
from clause is not only include relation, but also other plans, i think the
original name `from` is better than `relation`
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/analyzer/UnboundAlias.java:
##########
@@ -34,18 +38,28 @@
extends NamedExpression
implements UnaryExpression<CHILD_TYPE>, Unbound {
- public UnboundAlias(CHILD_TYPE child) {
+ private final ExprId exprId;
+ private final String name;
+
+ public UnboundAlias(ExprId exprId, CHILD_TYPE child) {
Review Comment:
unbound alias do not need ExprId at all, We only create unbound alias when
we cannot ensure this expression represent one column, so no slot reference
could refer to one UnboundAlias
--
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: commits-unsubscr...@doris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
