morrySnow commented on code in PR #41298: URL: https://github.com/apache/doris/pull/41298#discussion_r1800392729
########## fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindSink.java: ########## @@ -714,4 +741,65 @@ public Expression visitUnboundSlot(UnboundSlot unboundSlot, ExpressionRewriteCon return slotBinder.get(unboundSlot.getName()); } } + + private static class LegacyExprTranslator { + private final Map<String, Slot> nereidsSlotReplaceMap = new HashMap<>(); + private final OlapTable olapTable; + + public LegacyExprTranslator(OlapTable table, List<Slot> outputs) { + for (Slot expression : outputs) { + String name = expression.getName(); + nereidsSlotReplaceMap.put(name.toLowerCase(), expression.toSlot()); + } + this.olapTable = table; + } + + public List<Expression> createPartitionExprList() { + return olapTable.getPartitionInfo().getPartitionExprs().stream() + .map(expr -> analyze(expr.toSql())).collect(Collectors.toList()); Review Comment: could we not rely on `expr.toSql()`? because legacy planner's expr toSql is not right in some corner cases. the other way is fix all problem in `expr.toSql`. ########## fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/PlanTranslatorContext.java: ########## @@ -312,6 +312,26 @@ public SlotDescriptor createSlotDesc(TupleDescriptor tupleDesc, SlotReference sl return createSlotDesc(tupleDesc, slotReference, null); } + /** + * createSlotDescForSink + */ + public SlotDescriptor createSlotDescForSink(TupleDescriptor tupleDesc, SlotReference slotReference) { Review Comment: what the different between this and createSlotDesc? could we merge them into one function? ########## fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindSink.java: ########## @@ -218,7 +228,24 @@ private Plan bindOlapTableSink(MatchingContext<UnboundTableSink<Plan>> ctx) { ctx, table, isPartialUpdate, boundSink, child); LogicalProject<?> fullOutputProject = getOutputProjectByCoercion( table.getFullSchema(), child, columnToOutput); - return boundSink.withChildAndUpdateOutput(fullOutputProject); + List<Column> columns = new ArrayList<>(table.getFullSchema().size()); + for (int i = 0; i < table.getFullSchema().size(); ++i) { + Column col = table.getFullSchema().get(i); + if (columnToOutput.get(col.getName()) != null) { + columns.add(col); + } + } + Preconditions.checkArgument(fullOutputProject.getOutputs().size() == columns.size(), + "output's size should be same as columns's size"); Review Comment: we should throw AnalyzeExpression ########## fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindSink.java: ########## @@ -714,4 +741,65 @@ public Expression visitUnboundSlot(UnboundSlot unboundSlot, ExpressionRewriteCon return slotBinder.get(unboundSlot.getName()); } } + + private static class LegacyExprTranslator { + private final Map<String, Slot> nereidsSlotReplaceMap = new HashMap<>(); + private final OlapTable olapTable; + + public LegacyExprTranslator(OlapTable table, List<Slot> outputs) { + for (Slot expression : outputs) { + String name = expression.getName(); + nereidsSlotReplaceMap.put(name.toLowerCase(), expression.toSlot()); + } + this.olapTable = table; + } + + public List<Expression> createPartitionExprList() { + return olapTable.getPartitionInfo().getPartitionExprs().stream() + .map(expr -> analyze(expr.toSql())).collect(Collectors.toList()); + } + + public Map<Long, Expression> createSyncMvWhereClause() { + Map<Long, Expression> mvWhereClauses = new HashMap<>(); + long baseIndexId = olapTable.getBaseIndexId(); + for (Map.Entry<Long, MaterializedIndexMeta> entry : olapTable.getVisibleIndexIdToMeta().entrySet()) { + if (entry.getKey() != baseIndexId && entry.getValue().getWhereClause() != null) { + mvWhereClauses.put(entry.getKey(), analyze(entry.getValue().getWhereClause().toSql())); + } + } + return mvWhereClauses; + } + + private Expression analyze(String exprSql) { + Expression expression = new NereidsParser().parseExpression(exprSql); Review Comment: slotReplacer and a independent expression analyzer is little trick here. maybe we need to adjust analyze order to avoid analyze expression in mulit places? - bind relation - bind sink (create with unbound expression) - analyze expression - do sink cast or other things ########## fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/BindSink.java: ########## @@ -714,4 +741,65 @@ public Expression visitUnboundSlot(UnboundSlot unboundSlot, ExpressionRewriteCon return slotBinder.get(unboundSlot.getName()); } } + + private static class LegacyExprTranslator { + private final Map<String, Slot> nereidsSlotReplaceMap = new HashMap<>(); + private final OlapTable olapTable; + + public LegacyExprTranslator(OlapTable table, List<Slot> outputs) { + for (Slot expression : outputs) { + String name = expression.getName(); + nereidsSlotReplaceMap.put(name.toLowerCase(), expression.toSlot()); + } + this.olapTable = table; + } + + public List<Expression> createPartitionExprList() { + return olapTable.getPartitionInfo().getPartitionExprs().stream() + .map(expr -> analyze(expr.toSql())).collect(Collectors.toList()); + } + + public Map<Long, Expression> createSyncMvWhereClause() { + Map<Long, Expression> mvWhereClauses = new HashMap<>(); + long baseIndexId = olapTable.getBaseIndexId(); + for (Map.Entry<Long, MaterializedIndexMeta> entry : olapTable.getVisibleIndexIdToMeta().entrySet()) { + if (entry.getKey() != baseIndexId && entry.getValue().getWhereClause() != null) { + mvWhereClauses.put(entry.getKey(), analyze(entry.getValue().getWhereClause().toSql())); + } + } + return mvWhereClauses; + } + + private Expression analyze(String exprSql) { + Expression expression = new NereidsParser().parseExpression(exprSql); + Expression boundSlotExpression = + SlotReplacer.INSTANCE.replace(expression, nereidsSlotReplaceMap); + Scope scope = new Scope(Lists.newArrayList(nereidsSlotReplaceMap.values())); + LogicalEmptyRelation dummyPlan = new LogicalEmptyRelation( + ConnectContext.get().getStatementContext().getNextRelationId(), new ArrayList<>()); + CascadesContext cascadesContext = CascadesContext.initContext( + new StatementContext(ConnectContext.get(), new OriginStatement(exprSql, 0)), + dummyPlan, PhysicalProperties.ANY); Review Comment: why create a new cascadesContext here? why not use context for this statement? -- 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