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

Reply via email to