You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2020/01/02 15:04:23 UTC

[GitHub] [incubator-doris] xy720 opened a new pull request #2641: CreateViewStmt/AlterViewStmt support cte and fix bug

xy720 opened a new pull request #2641: CreateViewStmt/AlterViewStmt support cte and fix bug
URL: https://github.com/apache/incubator-doris/pull/2641
 
 
   This commit contains the following changes:
   1. Let create/alter view statement support cte sql. (Issue #2625 )
   e.g.
   ```
   Alter view test_tbl_view (h1, h2)
   as
   with testTbl_cte (w1, w2) as 
   (
       select col1, col2 from testDb.testTbl
   )
   select w1 as c1, sum(w2) as c2 from testTbl_cte 
   where w1 > 10 
   group by w1 order by w1
   ```
   
   2. Fix the bug that view's schema remains unchanged while replaying alter view. (Issue #2624 )

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] imay commented on a change in pull request #2641: CreateViewStmt/AlterViewStmt support cte and fix bug

Posted by GitBox <gi...@apache.org>.
imay commented on a change in pull request #2641: CreateViewStmt/AlterViewStmt support cte and fix bug
URL: https://github.com/apache/incubator-doris/pull/2641#discussion_r362783929
 
 

 ##########
 File path: fe/src/main/java/org/apache/doris/analysis/CreateViewStmt.java
 ##########
 @@ -55,21 +55,17 @@ public String getComment() {
 
     @Override
     public void analyze(Analyzer analyzer) throws AnalysisException, UserException {
-        if (cols != null) {
-            cloneStmt = viewDefStmt.clone();
-        }
         tableName.analyze(analyzer);
         viewDefStmt.setNeedToSql(true);
-        // Analyze view define statement
-        Analyzer viewAnalyzer = new Analyzer(analyzer);
-        viewDefStmt.analyze(viewAnalyzer);
 
         // check privilege
         if (!Catalog.getCurrentCatalog().getAuth().checkTblPriv(ConnectContext.get(), tableName.getDb(),
                                                                 tableName.getTbl(), PrivPredicate.CREATE)) {
             ErrorReport.reportAnalysisException(ErrorCode.ERR_SPECIFIC_ACCESS_DENIED_ERROR, "CREATE");
         }
 
+        // Analyze view define statement
+        viewDefStmt.analyze(analyzer);
 
 Review comment:
   should use a new Analyzer

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] xy720 commented on a change in pull request #2641: CreateViewStmt/AlterViewStmt support cte and fix bug

Posted by GitBox <gi...@apache.org>.
xy720 commented on a change in pull request #2641: CreateViewStmt/AlterViewStmt support cte and fix bug
URL: https://github.com/apache/incubator-doris/pull/2641#discussion_r363559114
 
 

 ##########
 File path: fe/src/main/java/org/apache/doris/analysis/CreateViewStmt.java
 ##########
 @@ -55,21 +55,17 @@ public String getComment() {
 
     @Override
     public void analyze(Analyzer analyzer) throws AnalysisException, UserException {
-        if (cols != null) {
-            cloneStmt = viewDefStmt.clone();
-        }
         tableName.analyze(analyzer);
         viewDefStmt.setNeedToSql(true);
-        // Analyze view define statement
-        Analyzer viewAnalyzer = new Analyzer(analyzer);
-        viewDefStmt.analyze(viewAnalyzer);
 
         // check privilege
         if (!Catalog.getCurrentCatalog().getAuth().checkTblPriv(ConnectContext.get(), tableName.getDb(),
                                                                 tableName.getTbl(), PrivPredicate.CREATE)) {
             ErrorReport.reportAnalysisException(ErrorCode.ERR_SPECIFIC_ACCESS_DENIED_ERROR, "CREATE");
         }
 
+        // Analyze view define statement
+        viewDefStmt.analyze(analyzer);
 
 Review comment:
   fixed

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] xy720 commented on a change in pull request #2641: CreateViewStmt/AlterViewStmt support cte and fix bug

Posted by GitBox <gi...@apache.org>.
xy720 commented on a change in pull request #2641: CreateViewStmt/AlterViewStmt support cte and fix bug
URL: https://github.com/apache/incubator-doris/pull/2641#discussion_r363559659
 
 

 ##########
 File path: fe/src/main/java/org/apache/doris/persist/AlterViewInfo.java
 ##########
 @@ -35,15 +38,18 @@
     private String inlineViewDef;
     @SerializedName(value = "sqlMode")
     private long sqlMode;
+    private List<Column> newFullSchema;
 
 Review comment:
   done

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] xy720 commented on a change in pull request #2641: CreateViewStmt/AlterViewStmt support cte and fix bug

Posted by GitBox <gi...@apache.org>.
xy720 commented on a change in pull request #2641: CreateViewStmt/AlterViewStmt support cte and fix bug
URL: https://github.com/apache/incubator-doris/pull/2641#discussion_r363560104
 
 

 ##########
 File path: fe/src/main/java/org/apache/doris/analysis/AlterViewStmt.java
 ##########
 @@ -54,20 +62,17 @@ public void analyze(Analyzer analyzer) throws AnalysisException, UserException {
                     tableName.getTbl());
         }
 
-        if (cols != null) {
-            cloneStmt = viewDefStmt.clone();
-        }
         viewDefStmt.setNeedToSql(true);
-        Analyzer viewAnalyzer = new Analyzer(analyzer);
+        viewDefStmt.analyze(analyzer);
 
 Review comment:
   fixed

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] imay commented on a change in pull request #2641: CreateViewStmt/AlterViewStmt support cte and fix bug

Posted by GitBox <gi...@apache.org>.
imay commented on a change in pull request #2641: CreateViewStmt/AlterViewStmt support cte and fix bug
URL: https://github.com/apache/incubator-doris/pull/2641#discussion_r362784750
 
 

 ##########
 File path: fe/src/main/java/org/apache/doris/analysis/BaseViewStmt.java
 ##########
 @@ -113,27 +113,21 @@ protected void createColumnAndViewDefs(Analyzer analyzer) throws AnalysisExcepti
             return;
         }
 
-        Analyzer tmpAnalyzer = new Analyzer(analyzer);
 
 Review comment:
   I'm not sure that the following will be right for every case.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] imay commented on a change in pull request #2641: CreateViewStmt/AlterViewStmt support cte and fix bug

Posted by GitBox <gi...@apache.org>.
imay commented on a change in pull request #2641: CreateViewStmt/AlterViewStmt support cte and fix bug
URL: https://github.com/apache/incubator-doris/pull/2641#discussion_r362785186
 
 

 ##########
 File path: fe/src/main/java/org/apache/doris/analysis/AlterViewStmt.java
 ##########
 @@ -54,20 +62,17 @@ public void analyze(Analyzer analyzer) throws AnalysisException, UserException {
                     tableName.getTbl());
         }
 
-        if (cols != null) {
-            cloneStmt = viewDefStmt.clone();
-        }
         viewDefStmt.setNeedToSql(true);
-        Analyzer viewAnalyzer = new Analyzer(analyzer);
+        viewDefStmt.analyze(analyzer);
 
 Review comment:
   must analyze view with a new analyzer.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] imay commented on a change in pull request #2641: CreateViewStmt/AlterViewStmt support cte and fix bug

Posted by GitBox <gi...@apache.org>.
imay commented on a change in pull request #2641: CreateViewStmt/AlterViewStmt support cte and fix bug
URL: https://github.com/apache/incubator-doris/pull/2641#discussion_r362785091
 
 

 ##########
 File path: fe/src/main/java/org/apache/doris/analysis/BaseViewStmt.java
 ##########
 @@ -113,27 +113,21 @@ protected void createColumnAndViewDefs(Analyzer analyzer) throws AnalysisExcepti
             return;
         }
 
-        Analyzer tmpAnalyzer = new Analyzer(analyzer);
-        List<String> colNames = cols.stream().map(c -> c.getColName()).collect(Collectors.toList());
-        cloneStmt.substituteSelectList(tmpAnalyzer, colNames);
-        inlineViewDef = cloneStmt.toSql();
-
-        // StringBuilder sb = new StringBuilder();
-        // sb.append("SELECT ");
-        // for (int i = 0; i < columnNames.size(); ++i) {
-        //     if (i != 0) {
-        //         sb.append(", ");
-        //     }
-        //     String colRef = viewDefStmt.getColLabels().get(i);
-        //     if (!colRef.startsWith("`")) {
-        //         colRef = "`" + colRef + "`";
-        //     }
-        //     String colAlias = finalCols.get(i).getName();
-
-        //     sb.append(String.format("`%s`.%s AS `%s`", tableName.getTbl(), colRef, colAlias));
-        // }
-        // sb.append(String.format(" FROM (%s) %s", originalViewDef, tableName.getTbl()));
-        // inlineViewDef = sb.toString();
+        StringBuilder sb = new StringBuilder();
+        sb.append("SELECT ");
+        for (int i = 0; i < finalCols.size(); ++i) {
+            if (i != 0) {
+                sb.append(", ");
+            }
+            String colRef = viewDefStmt.getColLabels().get(i);
+            if (!colRef.startsWith("`")) {
+                colRef = "`" + colRef + "`";
+            }
+            String colAlias = finalCols.get(i).getName();
+            sb.append(String.format("`%s`.%s AS `%s`", tableName.getTbl(), colRef, colAlias));
 
 Review comment:
   Is this work for a function call in select list?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] morningman commented on a change in pull request #2641: CreateViewStmt/AlterViewStmt support cte and fix bug

Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #2641: CreateViewStmt/AlterViewStmt support cte and fix bug
URL: https://github.com/apache/incubator-doris/pull/2641#discussion_r363024459
 
 

 ##########
 File path: fe/src/main/java/org/apache/doris/persist/AlterViewInfo.java
 ##########
 @@ -35,15 +38,18 @@
     private String inlineViewDef;
     @SerializedName(value = "sqlMode")
     private long sqlMode;
+    private List<Column> newFullSchema;
 
 Review comment:
   You can also add @SerializedName to `newFullSchema` 

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] xy720 commented on a change in pull request #2641: CreateViewStmt/AlterViewStmt support cte and fix bug

Posted by GitBox <gi...@apache.org>.
xy720 commented on a change in pull request #2641: CreateViewStmt/AlterViewStmt support cte and fix bug
URL: https://github.com/apache/incubator-doris/pull/2641#discussion_r363181193
 
 

 ##########
 File path: fe/src/main/java/org/apache/doris/analysis/BaseViewStmt.java
 ##########
 @@ -113,27 +113,21 @@ protected void createColumnAndViewDefs(Analyzer analyzer) throws AnalysisExcepti
             return;
         }
 
-        Analyzer tmpAnalyzer = new Analyzer(analyzer);
-        List<String> colNames = cols.stream().map(c -> c.getColName()).collect(Collectors.toList());
-        cloneStmt.substituteSelectList(tmpAnalyzer, colNames);
-        inlineViewDef = cloneStmt.toSql();
-
-        // StringBuilder sb = new StringBuilder();
-        // sb.append("SELECT ");
-        // for (int i = 0; i < columnNames.size(); ++i) {
-        //     if (i != 0) {
-        //         sb.append(", ");
-        //     }
-        //     String colRef = viewDefStmt.getColLabels().get(i);
-        //     if (!colRef.startsWith("`")) {
-        //         colRef = "`" + colRef + "`";
-        //     }
-        //     String colAlias = finalCols.get(i).getName();
-
-        //     sb.append(String.format("`%s`.%s AS `%s`", tableName.getTbl(), colRef, colAlias));
-        // }
-        // sb.append(String.format(" FROM (%s) %s", originalViewDef, tableName.getTbl()));
-        // inlineViewDef = sb.toString();
+        StringBuilder sb = new StringBuilder();
+        sb.append("SELECT ");
+        for (int i = 0; i < finalCols.size(); ++i) {
+            if (i != 0) {
+                sb.append(", ");
+            }
+            String colRef = viewDefStmt.getColLabels().get(i);
+            if (!colRef.startsWith("`")) {
+                colRef = "`" + colRef + "`";
+            }
+            String colAlias = finalCols.get(i).getName();
+            sb.append(String.format("`%s`.%s AS `%s`", tableName.getTbl(), colRef, colAlias));
 
 Review comment:
   It is work. Because the analyze is happen at before. What is doing here is just to wrap another sql outside the previous analyzed sql.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] imay merged pull request #2641: CreateViewStmt/AlterViewStmt support cte and fix bug

Posted by GitBox <gi...@apache.org>.
imay merged pull request #2641: CreateViewStmt/AlterViewStmt support cte and fix bug
URL: https://github.com/apache/incubator-doris/pull/2641
 
 
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org