[Fix](nereids) fix create view and alter view using old parser checki…

…ng (#43263) Problem: Creating a view on a table with materialized views (MV) results in an error: failed to init view stmt. For example, executing the following SQL: create view v1 as select * from t1 index t_mv_mv. Root Cause: The syntax index t_mv_mv is supported only by the new planner and not by the old planner. During the view creation process, regardless of whether the new or old optimizer is used, the process eventually calls the createView method in Env. In the createView method, the persisted SQL of the view is parsed using the old optimizer's parser. If the parsing fails, an error is thrown. As a result, if the SQL for creating a view includes syntax supported only by the new optimizer, an error occurs. Solution: This PR addresses the issue by removing the parsing check from the createView method in Env. Instead, the check is moved to the respective classes responsible for creating views in the new and old optimizers.
apache · Nov 22, 2024 · 21eb247 · 21eb247
1 parent ce55752
commit 21eb247
Show file tree

Hide file tree

Showing 11 changed files with 548 additions and 423 deletions.
diff --git a/fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java b/fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java
@@ -693,11 +693,6 @@ private void modifyViewDef(Database db, View view, String inlineViewDef, long sq
             view.writeLockOrDdlException();
             try {
                 view.setInlineViewDefWithSqlMode(inlineViewDef, sqlMode);
-                try {
-                    view.init();
-                } catch (UserException e) {
-                    throw new DdlException("failed to init view stmt, reason=" + e.getMessage());
-                }
                 view.setNewFullSchema(newFullSchema);
                 String viewName = view.getName();
                 db.unregisterTable(viewName);

diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/BaseViewStmt.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/BaseViewStmt.java
@@ -21,20 +21,24 @@
 import org.apache.doris.catalog.Env;
 import org.apache.doris.catalog.Type;
 import org.apache.doris.common.AnalysisException;
+import org.apache.doris.common.DdlException;
 import org.apache.doris.common.ErrorCode;
 import org.apache.doris.common.ErrorReport;
 import org.apache.doris.common.UserException;
+import org.apache.doris.common.util.SqlParserUtils;
 import org.apache.doris.common.util.ToSqlContext;
 import org.apache.doris.datasource.InternalCatalog;
 import org.apache.doris.mysql.privilege.PrivPredicate;
 import org.apache.doris.qe.ConnectContext;
 
+import com.google.common.base.Preconditions;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Sets;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 
+import java.io.StringReader;
 import java.util.List;
 import java.util.Set;
 import java.util.stream.Collectors;
@@ -147,6 +151,7 @@ protected void createColumnAndViewDefs(Analyzer analyzer) throws AnalysisExcepti
                 // we don't need the slot id info, so using ToSqlContext to remove it.
                 toSqlContext.setNeedSlotRefId(false);
                 inlineViewDef = viewDefStmt.toSql();
+                checkInlineViewDef();
             }
             return;
         }
@@ -161,6 +166,7 @@ protected void createColumnAndViewDefs(Analyzer analyzer) throws AnalysisExcepti
             // we don't need the slot id info, so using ToSqlContext to remove it.
             toSqlContext.setNeedSlotRefId(false);
             inlineViewDef = cloneStmt.toSql();
+            checkInlineViewDef();
         }
     }
 
@@ -172,4 +178,24 @@ public void analyze(Analyzer analyzer) throws AnalysisException, UserException {
             throw new AnalysisException("Not support OUTFILE clause in CREATE VIEW statement");
         }
     }
+
+    private void checkInlineViewDef() throws UserException {
+        Preconditions.checkNotNull(inlineViewDef);
+        SqlScanner input = new SqlScanner(new StringReader(inlineViewDef),
+                ConnectContext.get().getSessionVariable().getSqlMode());
+        SqlParser parser = new SqlParser(input);
+        ParseNode node;
+        try {
+            node = SqlParserUtils.getFirstStmt(parser);
+        } catch (Exception e) {
+            throw new DdlException(
+                    String.format("Failed to parse view-definition statement of view: %s, stmt is %s, reason is %s",
+                            tableName, inlineViewDef, e.getMessage()));
+        }
+        // Make sure the view definition parses to a query statement.
+        if (!(node instanceof QueryStmt)) {
+            throw new DdlException(String.format("View definition of %s "
+                    + "is not a query statement", tableName));
+        }
+    }
 }
diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/Env.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/Env.java
@@ -5635,13 +5635,6 @@ public void createView(CreateViewStmt stmt) throws DdlException {
             newView.setComment(stmt.getComment());
             newView.setInlineViewDefWithSqlMode(stmt.getInlineViewDef(),
                     ConnectContext.get().getSessionVariable().getSqlMode());
-            // init here in case the stmt string from view.toSql() has some syntax error.
-            try {
-                newView.init();
-            } catch (UserException e) {
-                throw new DdlException("failed to init view stmt, reason=" + e.getMessage());
-            }
-
             if (!((Database) db).createTableWithLock(newView, false, stmt.isSetIfNotExists()).first) {
                 throw new DdlException("Failed to create view[" + tableName + "].");
             }

diff --git a/...-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/AlterViewInfo.java b/...-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/AlterViewInfo.java
@@ -78,18 +78,16 @@ public void init(ConnectContext ctx) throws UserException {
 
     /**translateToLegacyStmt*/
     public AlterViewStmt translateToLegacyStmt(ConnectContext ctx) {
-        List<ColWithComment> cols = Lists.newArrayList();
-        for (SimpleColumnDefinition def : simpleColumnDefinitions) {
-            cols.add(def.translateToColWithComment());
-        }
-        AlterViewStmt alterViewStmt = new AlterViewStmt(viewName.transferToTableName(), cols,
-                null);
         // expand star(*) in project list and replace table name with qualifier
         String rewrittenSql = rewriteSql(ctx.getStatementContext().getIndexInSqlToString(), querySql);
-
         // rewrite project alias
         rewrittenSql = rewriteProjectsToUserDefineAlias(rewrittenSql);
-
+        checkViewSql(rewrittenSql);
+        List<ColWithComment> cols = Lists.newArrayList();
+        for (SimpleColumnDefinition def : simpleColumnDefinitions) {
+            cols.add(def.translateToColWithComment());
+        }
+        AlterViewStmt alterViewStmt = new AlterViewStmt(viewName.transferToTableName(), cols, null);
         alterViewStmt.setInlineViewDef(rewrittenSql);
         alterViewStmt.setFinalColumns(finalCols);
         return alterViewStmt;

diff --git a/...e-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/BaseViewInfo.java b/...e-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/BaseViewInfo.java
@@ -389,4 +389,8 @@ public Void visitSlot(Slot slot, StatementContext ctx) {
             return null;
         }
     }
+
+    protected void checkViewSql(String viewSql) {
+        new NereidsParser().parseSingle(viewSql);
+    }
 }
diff --git a/...core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/CreateViewInfo.java b/...core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/CreateViewInfo.java
@@ -73,18 +73,17 @@ public void init(ConnectContext ctx) throws UserException {
 
     /**translateToLegacyStmt*/
     public CreateViewStmt translateToLegacyStmt(ConnectContext ctx) {
+        // expand star(*) in project list and replace table name with qualifier
+        String rewrittenSql = rewriteSql(ctx.getStatementContext().getIndexInSqlToString(), querySql);
+        // rewrite project alias
+        rewrittenSql = rewriteProjectsToUserDefineAlias(rewrittenSql);
+        checkViewSql(rewrittenSql);
         List<ColWithComment> cols = Lists.newArrayList();
         for (SimpleColumnDefinition def : simpleColumnDefinitions) {
             cols.add(def.translateToColWithComment());
         }
         CreateViewStmt createViewStmt = new CreateViewStmt(ifNotExists, orReplace, viewName.transferToTableName(), cols,
                 comment, null);
-        // expand star(*) in project list and replace table name with qualifier
-        String rewrittenSql = rewriteSql(ctx.getStatementContext().getIndexInSqlToString(), querySql);
-
-        // rewrite project alias
-        rewrittenSql = rewriteProjectsToUserDefineAlias(rewrittenSql);
-
         createViewStmt.setInlineViewDef(rewrittenSql);
         createViewStmt.setFinalColumns(finalCols);
         return createViewStmt;

diff --git a/regression-test/data/ddl_p0/create_view_nereids/create_view_use_mv.out b/regression-test/data/ddl_p0/create_view_nereids/create_view_use_mv.out
@@ -0,0 +1,15 @@
+-- This file is automatically generated. You should know what you did if you want to edit this
+-- !create_view_from_mv --
+\N	99.50	99.50	99.50	1	\N	\N	\N	\N	1	\N	\N
+1	208.70	109.20	99.50	3	1	3	1	1	3	\N	\N
+2	218.40	109.20	109.20	2	2	2	2	2	1	\N	\N
+3	298.50	99.50	99.50	3	3	6	3	3	2	\N	\N
+4	99.50	99.50	99.50	1	\N	4	4	4	1	\N	\N
+
+-- !alter_view_from_mv --
+\N	99.50	99.50	99.50	1	\N	\N	\N	\N	1	\N	\N
+1	208.70	109.20	99.50	3	1	3	1	1	3	\N	\N
+2	218.40	109.20	109.20	2	2	2	2	2	1	\N	\N
+3	298.50	99.50	99.50	3	3	6	3	3	2	\N	\N
+4	99.50	99.50	99.50	1	\N	4	4	4	1	\N	\N
+