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: [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]
