You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2023/01/08 14:01:24 UTC
[GitHub] [doris] starocean999 opened a new pull request, #15714: [fix](nereids)fix some nereids bugs
starocean999 opened a new pull request, #15714:
URL: https://github.com/apache/doris/pull/15714
# Proposed changes
Issue Number: close #xxx
## Problem summary
1.remove any project list from EmptySetNode.
2.order by xxx desc should use nulls last as default order.
3.don't create runtime filter if runtime filter mode is OFF.
4.group by constant value need check the corresponding expr shouldn't have any aggregation functions.
5.fix two left outer join reorder bug( A left join B left join C).
6.fix semi join and left outer join reorder bug.( A left join B semi join C ).
7.fix group by NULL bug.
8.change ceil and floor function to correct signature.
9.add literal comparasion for string and date type.
10.fix the getOnClauseUsedSlots method may not return valid value.
11.the tightness common type of string and date should be date.
12.the nullability of set operation node's result exprs is not set correctly.
13.Sort node should remove redundent ordering exprs.
## Checklist(Required)
1. Does it affect the original behavior:
- [ ] Yes
- [ ] No
- [ ] I don't know
2. Has unit tests been added:
- [ ] Yes
- [ ] No
- [ ] No Need
3. Has document been added or modified:
- [ ] Yes
- [ ] No
- [ ] No Need
4. Does it need to update dependencies:
- [ ] Yes
- [ ] No
5. Are there any changes that cannot be rolled back:
- [ ] Yes (If Yes, please explain WHY)
- [ ] No
## Further comments
If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] morrySnow commented on a diff in pull request #15714: [fix](nereids)fix some nereids bugs
Posted by GitBox <gi...@apache.org>.
morrySnow commented on code in PR #15714:
URL: https://github.com/apache/doris/pull/15714#discussion_r1065460283
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/ResolveOrdinalInOrderByAndGroupBy.java:
##########
@@ -74,6 +76,12 @@ public List<Rule> buildRules() {
int ord = i.getIntValue();
checkOrd(ord, aggOutput.size());
Expression aggExpr = aggOutput.get(ord - 1);
+ if (aggExpr instanceof Alias) {
+ aggExpr = ((Alias) aggExpr).child();
+ if (aggExpr instanceof Literal) {
+ aggExpr = groupByExpr;
+ }
Review Comment:
we should replaced by expression in output list in any case
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] hello-stephen commented on pull request #15714: [fix](nereids)fix some nereids bugs
Posted by GitBox <gi...@apache.org>.
hello-stephen commented on PR #15714:
URL: https://github.com/apache/doris/pull/15714#issuecomment-1374848616
TeamCity pipeline, clickbench performance test result:
the sum of best hot time: 34.93 seconds
load time: 479 seconds
storage size: 17121594761 Bytes
https://doris-community-test-1308700295.cos.ap-hongkong.myqcloud.com/tmp/20230108142350_clickbench_pr_75649.html
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #15714: [fix](nereids)fix some nereids bugs
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #15714:
URL: https://github.com/apache/doris/pull/15714#issuecomment-1378450288
PR approved by at least one committer and no changes requested.
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] jackwener commented on a diff in pull request #15714: [fix](nereids)fix some nereids bugs
Posted by GitBox <gi...@apache.org>.
jackwener commented on code in PR #15714:
URL: https://github.com/apache/doris/pull/15714#discussion_r1065466727
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/join/SemiJoinLogicalJoinTranspose.java:
##########
@@ -97,13 +104,17 @@ public Rule build() {
* / \ / \
* A B B C
*/
- LogicalJoin<GroupPlan, GroupPlan> newBottomSemiJoin = new LogicalJoin<>(
- topSemiJoin.getJoinType(),
- topSemiJoin.getHashJoinConjuncts(), topSemiJoin.getOtherJoinConjuncts(),
- JoinHint.NONE,
- b, c);
- return new LogicalJoin<>(bottomJoin.getJoinType(), bottomJoin.getHashJoinConjuncts(),
- bottomJoin.getOtherJoinConjuncts(), JoinHint.NONE, a, newBottomSemiJoin);
+ if (bottomJoinType.isInnerJoin() || bottomJoinType.isLeftOuterJoin()) {
Review Comment:
Add them into `typeCheck()` and put it into `when()`
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/join/OuterJoinLAsscomProject.java:
##########
@@ -200,4 +196,35 @@ public Rule build() {
return PlanUtils.project(new ArrayList<>(topJoin.getOutput()), newTopJoin).get();
}).toRule(RuleType.LOGICAL_OUTER_JOIN_LASSCOM_PROJECT);
}
+
+ private Map<Boolean, List<Expression>> splitConjunctsWithAlias(List<Expression> topConjuncts,
Review Comment:
rename it.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/join/SemiJoinLogicalJoinTransposeProject.java:
##########
@@ -109,15 +116,20 @@ public Rule build() {
* / \ / \
* A B B C
*/
- LogicalJoin<GroupPlan, GroupPlan> newBottomSemiJoin = new LogicalJoin<>(
- topSemiJoin.getJoinType(), topSemiJoin.getHashJoinConjuncts(),
- topSemiJoin.getOtherJoinConjuncts(), JoinHint.NONE, b, c);
-
- LogicalJoin<Plan, Plan> newTopJoin = new LogicalJoin<>(bottomJoin.getJoinType(),
- bottomJoin.getHashJoinConjuncts(), bottomJoin.getOtherJoinConjuncts(), JoinHint.NONE,
- a, newBottomSemiJoin);
-
- return new LogicalProject<>(new ArrayList<>(topSemiJoin.getOutput()), newTopJoin);
+ if (bottomJoinType.isInnerJoin() || bottomJoinType.isLeftOuterJoin()) {
Review Comment:
ditto
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] morrySnow commented on a diff in pull request #15714: [fix](nereids)fix some nereids bugs
Posted by GitBox <gi...@apache.org>.
morrySnow commented on code in PR #15714:
URL: https://github.com/apache/doris/pull/15714#discussion_r1064366410
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PhysicalPlanTranslator.java:
##########
@@ -1102,10 +1102,13 @@ public PlanFragment visitPhysicalProject(PhysicalProject<? extends Plan> project
}
TupleDescriptor tupleDescriptor = generateTupleDesc(slotList, null, context);
- inputPlanNode.setProjectList(execExprList);
+ if (!(inputPlanNode instanceof EmptySetNode)) {
+ inputPlanNode.setProjectList(execExprList);
+ }
Review Comment:
add a todo, we need to merge project(emptyset) to a new emptyset in rewrite step of Nereids
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/ResolveOrdinalInOrderByAndGroupBy.java:
##########
@@ -74,6 +76,11 @@ public List<Rule> buildRules() {
int ord = i.getIntValue();
checkOrd(ord, aggOutput.size());
Expression aggExpr = aggOutput.get(ord - 1);
+ if (aggExpr.containsType(AggregateFunction.class)) {
+ throw new AnalysisException(
+ "GROUP BY expression must not contain aggregate functions: "
+ + aggExpr.toSql());
+ }
Review Comment:
we should not add check here, instead we need to add a check in CheckAnalysis
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/logical/NormalizeToSlot.java:
##########
@@ -133,7 +136,10 @@ public static NormalizeToSlotTriplet toTriplet(Expression expression, @Nullable
}
Alias alias = new Alias(expression, expression.toSql());
- return new NormalizeToSlotTriplet(expression, alias.toSlot(), alias);
+ SlotReference slot = (SlotReference) alias.toSlot();
+ // BE will create a nullable uint8 column to expand NullLiteral when necessary
+ return new NormalizeToSlotTriplet(expression, expression instanceof NullLiteral ? slot.withDataType(
+ TinyIntType.INSTANCE) : slot, alias);
Review Comment:
if we just want to process `group by null`, maybe a better way to do it is generating NullLiteral with TinyIntType when we do rewrite.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/util/JoinUtils.java:
##########
@@ -151,11 +152,14 @@ public static Pair<List<ExprId>, List<ExprId>> getOnClauseUsedSlots(
for (Expression expr : join.getHashJoinConjuncts()) {
EqualTo equalTo = (EqualTo) expr;
- if (!(equalTo.left() instanceof Slot) || !(equalTo.right() instanceof Slot)) {
+ Optional<Slot> leftSlot = ExpressionUtils.extractSlotOrCastOnSlot(equalTo.left());
+ Optional<Slot> rightSlot = ExpressionUtils.extractSlotOrCastOnSlot(equalTo.right());
+ if (!leftSlot.isPresent() || !rightSlot.isPresent()) {
Review Comment:
add a TODO on it, to explain why and we will fix normalize join hash equals future
##########
fe/fe-core/src/main/java/org/apache/doris/planner/SortNode.java:
##########
@@ -320,7 +320,12 @@ public Set<SlotId> computeInputSlotIds(Analyzer analyzer) throws NotImplementedE
*/
public void finalizeForNereids(TupleDescriptor tupleDescriptor,
List<Expr> outputList, List<Expr> orderingExpr) {
- resolvedTupleExprs = Lists.newArrayList(orderingExpr);
+ resolvedTupleExprs = Lists.newArrayList();
+ for (Expr order : orderingExpr) {
+ if (!resolvedTupleExprs.contains(order)) {
+ resolvedTupleExprs.add(order);
+ }
+ }
Review Comment:
add a TODO, we need to fix it in Nereids' code later
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/util/TypeCoercionUtils.java:
##########
@@ -215,6 +215,9 @@ public static Optional<DataType> findTightestCommonType(BinaryOperator binaryOpe
}
} else if (left instanceof CharacterType && right instanceof CharacterType) {
tightestCommonType = CharacterType.widerCharacterType((CharacterType) left, (CharacterType) right);
+ } else if (left instanceof CharacterType && right instanceof DateLikeType) {
+ // TODO: need check implicitCastMap to keep the behavior consistent with old optimizer
+ tightestCommonType = right;
Review Comment:
should we also add `right instanceof CharacterType && left instanceof DateLikeType`
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] jackwener commented on a diff in pull request #15714: [fix](nereids)fix some nereids bugs
Posted by GitBox <gi...@apache.org>.
jackwener commented on code in PR #15714:
URL: https://github.com/apache/doris/pull/15714#discussion_r1065471589
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/join/SemiJoinLogicalJoinTranspose.java:
##########
@@ -83,12 +85,17 @@ public Rule build() {
* / \ / \
* A B A C
*/
- LogicalJoin<GroupPlan, GroupPlan> newBottomSemiJoin = new LogicalJoin<>(
- topSemiJoin.getJoinType(),
- topSemiJoin.getHashJoinConjuncts(), topSemiJoin.getOtherJoinConjuncts(), JoinHint.NONE,
- a, c);
- return new LogicalJoin<>(bottomJoin.getJoinType(), bottomJoin.getHashJoinConjuncts(),
- bottomJoin.getOtherJoinConjuncts(), JoinHint.NONE, newBottomSemiJoin, b);
+ if (bottomJoinType.isInnerJoin() || bottomJoinType.isRightOuterJoin()) {
+ LogicalJoin<GroupPlan, GroupPlan> newBottomSemiJoin = new LogicalJoin<>(
+ topSemiJoin.getJoinType(),
+ topSemiJoin.getHashJoinConjuncts(), topSemiJoin.getOtherJoinConjuncts(),
+ JoinHint.NONE,
+ a, c);
+ return new LogicalJoin<>(JoinType.INNER_JOIN, bottomJoin.getHashJoinConjuncts(),
Review Comment:
return new LogicalJoin<>(bottomJoinType, bottomJoin.getHashJoinConjuncts(),
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/join/SemiJoinLogicalJoinTranspose.java:
##########
@@ -75,6 +75,8 @@ public Rule build() {
lasscom = ExpressionUtils.isIntersecting(usedSlot, aOutputSet) || lasscom;
}
+ JoinType bottomJoinType = bottomJoin.getJoinType();
+
Review Comment:
JoinType newBottomJoinType = JoinType.INNER_JOIN;
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/join/SemiJoinLogicalJoinTranspose.java:
##########
@@ -83,12 +85,17 @@ public Rule build() {
* / \ / \
* A B A C
*/
- LogicalJoin<GroupPlan, GroupPlan> newBottomSemiJoin = new LogicalJoin<>(
- topSemiJoin.getJoinType(),
- topSemiJoin.getHashJoinConjuncts(), topSemiJoin.getOtherJoinConjuncts(), JoinHint.NONE,
- a, c);
- return new LogicalJoin<>(bottomJoin.getJoinType(), bottomJoin.getHashJoinConjuncts(),
- bottomJoin.getOtherJoinConjuncts(), JoinHint.NONE, newBottomSemiJoin, b);
+ if (bottomJoinType.isInnerJoin() || bottomJoinType.isRightOuterJoin()) {
+ LogicalJoin<GroupPlan, GroupPlan> newBottomSemiJoin = new LogicalJoin<>(
+ topSemiJoin.getJoinType(),
+ topSemiJoin.getHashJoinConjuncts(), topSemiJoin.getOtherJoinConjuncts(),
+ JoinHint.NONE,
+ a, c);
+ return new LogicalJoin<>(JoinType.INNER_JOIN, bottomJoin.getHashJoinConjuncts(),
Review Comment:
return new LogicalJoin<>(bottomJoinType,
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/exploration/join/SemiJoinLogicalJoinTranspose.java:
##########
@@ -83,12 +85,17 @@ public Rule build() {
* / \ / \
* A B A C
*/
- LogicalJoin<GroupPlan, GroupPlan> newBottomSemiJoin = new LogicalJoin<>(
- topSemiJoin.getJoinType(),
- topSemiJoin.getHashJoinConjuncts(), topSemiJoin.getOtherJoinConjuncts(), JoinHint.NONE,
- a, c);
- return new LogicalJoin<>(bottomJoin.getJoinType(), bottomJoin.getHashJoinConjuncts(),
- bottomJoin.getOtherJoinConjuncts(), JoinHint.NONE, newBottomSemiJoin, b);
+ if (bottomJoinType.isInnerJoin() || bottomJoinType.isRightOuterJoin()) {
+ LogicalJoin<GroupPlan, GroupPlan> newBottomSemiJoin = new LogicalJoin<>(
+ topSemiJoin.getJoinType(),
+ topSemiJoin.getHashJoinConjuncts(), topSemiJoin.getOtherJoinConjuncts(),
+ JoinHint.NONE,
+ a, c);
+ return new LogicalJoin<>(JoinType.INNER_JOIN, bottomJoin.getHashJoinConjuncts(),
Review Comment:
return new LogicalJoin<>(bottomJoinType,
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] github-actions[bot] commented on pull request #15714: [fix](nereids)fix some nereids bugs
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #15714:
URL: https://github.com/apache/doris/pull/15714#issuecomment-1378450357
PR approved by anyone and no changes requested.
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
[GitHub] [doris] morrySnow merged pull request #15714: [fix](nereids)fix some nereids bugs
Posted by GitBox <gi...@apache.org>.
morrySnow merged PR #15714:
URL: https://github.com/apache/doris/pull/15714
--
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-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org