EmmyMiao87 commented on a change in pull request #3677: URL: https://github.com/apache/incubator-doris/pull/3677#discussion_r455005784
########## File path: fe/src/main/java/org/apache/doris/analysis/InsertStmt.java ########## @@ -437,15 +438,36 @@ private void analyzeSubquery(Analyzer analyzer) throws UserException { * Rule A: If the column which the shadow column related to is not mentioned, * then do not add the shadow column to targetColumns. They will be filled by * null or default value when loading. + * + * When table have materialized view, there may be some materialized view columns. + * we should add them to the end of targetColumns. + * eg: origin targetColumns: (A,B,C), shadow column: __doris_materialized_view_bitmap_union_C Review comment: ```suggestion * eg: origin targetColumns: (A,B,C), shadow column: mv_bitmap_union_C ``` ########## File path: fe/src/main/java/org/apache/doris/analysis/InsertStmt.java ########## @@ -437,15 +438,36 @@ private void analyzeSubquery(Analyzer analyzer) throws UserException { * Rule A: If the column which the shadow column related to is not mentioned, * then do not add the shadow column to targetColumns. They will be filled by * null or default value when loading. + * + * When table have materialized view, there may be some materialized view columns. + * we should add them to the end of targetColumns. + * eg: origin targetColumns: (A,B,C), shadow column: __doris_materialized_view_bitmap_union_C + * after processing, targetColumns: (A, B, C, __doris_materialized_view_bitmap_union_C), and Review comment: same as above ########## File path: fe/src/main/java/org/apache/doris/analysis/InsertStmt.java ########## @@ -518,17 +550,26 @@ private void analyzeSubquery(Analyzer analyzer) throws UserException { } } - // expand baseTblResultExprs and colLabels in QueryStmt + // expand colLabels in QueryStmt if (!origColIdxsForShadowCols.isEmpty()) { if (queryStmt.getResultExprs().size() != queryStmt.getBaseTblResultExprs().size()) { - for (Integer idx : origColIdxsForShadowCols) { - queryStmt.getBaseTblResultExprs().add(queryStmt.getBaseTblResultExprs().get(idx)); + for (Pair<Integer, Column> entry : origColIdxsForShadowCols) { Review comment: Totally same as the result expr of query stmt? ########## File path: fe/src/main/java/org/apache/doris/analysis/InsertStmt.java ########## @@ -437,15 +438,36 @@ private void analyzeSubquery(Analyzer analyzer) throws UserException { * Rule A: If the column which the shadow column related to is not mentioned, * then do not add the shadow column to targetColumns. They will be filled by * null or default value when loading. + * + * When table have materialized view, there may be some materialized view columns. + * we should add them to the end of targetColumns. + * eg: origin targetColumns: (A,B,C), shadow column: __doris_materialized_view_bitmap_union_C + * after processing, targetColumns: (A, B, C, __doris_materialized_view_bitmap_union_C), and + * origColIdx2MVColumn has 1 element: "2, __doris_materialized_view_bitmap_union_C" + * will be used in as a mapping from queryStmt.getResultExprs() to targetColumns define expr */ - List<Integer> origColIdxsForShadowCols = Lists.newArrayList(); + List<Pair<Integer, Column>> origColIdxsForShadowCols = Lists.newArrayList(); Review comment: The shadow cols is not meaningful. Maybe extend ? ---------------------------------------------------------------- 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. 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