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 2022/06/30 09:53:55 UTC

[GitHub] [doris] AshinGau opened a new pull request, #10521: [feature-wip](multi-catalog) end to end to support multi-catalog

AshinGau opened a new pull request, #10521:
URL: https://github.com/apache/doris/pull/10521

   ## What is changed
   Get through the previous pull requests that support multi-catalog, and end to end to achieve multi-catalog.
   
   ## Usage
   ### Add catalog
   ```
   MySQL [tpch1]> create catalog hive properties('type' = 'hms', 'hive.metastore.uris' = 'thrift://172.21.16.34:7004');
   
   MySQL [tpch1]> show catalogs;
   +------------------+----------+
   | CatalogName      | Type     |
   +------------------+----------+
   | hive             | hms      |
   | internal_catalog | internal |
   +------------------+----------+
   ```
   ### Switch catalog & Describe hive table
   ```
   MySQL [tpch1]> switch hive;
   Query OK, 0 rows affected (0.00 sec)
   MySQL [tpch1]> show databases;
   +---------------+
   | Database      |
   +---------------+
   | cmy           |
   | db            |
   | default       |
   | tpch1         |
   | tpch100       |
   | tpch1_parquet |
   +---------------+
   MySQL [tpch1]> use tpch1_parquet;
   Database changed
   MySQL [tpch1_parquet]> show tables;
   +-------------------------+
   | Tables_in_tpch1_parquet |
   +-------------------------+
   | customer                |
   | lineitem                |
   | nation                  |
   | orders                  |
   | part                    |
   | partsupp                |
   | region                  |
   | supplier                |
   +-------------------------+
   MySQL [tpch1_parquet]> describe nation;
   +-------------+---------+---------+
   | Field       | Type    | Comment |
   +-------------+---------+---------+
   | n_nationkey | int(11) | NULL    |
   | n_name      | text    | NULL    |
   | n_regionkey | int(11) | NULL    |
   | n_comment   | text    | NULL    |
   +-------------+---------+---------+
   ```
   ### Query
   ```
   MySQL [tpch1_parquet]> select count(*) from nation;
   +----------+
   | count(*) |
   +----------+
   |       25 |
   +----------+
   ```
   
   
   ## Checklist(Required)
   
   1. Does it affect the original behavior: (No)
   2. Has unit tests been added: (No)
   3. Has document been added or modified: (No)
   4. Does it need to update dependencies: (No)
   5. Are there any changes that cannot be rolled back: (No)
   
   ## Further comments
   
   If this is a relatively large or complex change, kick off the discussion at [dev@doris.apache.org](mailto:dev@doris.apache.org) by explaining why you chose the solution you did and what alternatives you considered, etc...
   


-- 
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-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [doris] morningman merged pull request #10521: [feature-wip](multi-catalog) end to end to support multi-catalog

Posted by GitBox <gi...@apache.org>.
morningman merged PR #10521:
URL: https://github.com/apache/doris/pull/10521


-- 
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-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [doris] morningman commented on a diff in pull request #10521: [feature-wip](multi-catalog) end to end to support multi-catalog

Posted by GitBox <gi...@apache.org>.
morningman commented on code in PR #10521:
URL: https://github.com/apache/doris/pull/10521#discussion_r910840777


##########
fe/fe-core/src/main/java/org/apache/doris/qe/ShowExecutor.java:
##########
@@ -647,7 +650,13 @@ private void handleShowPartitionId() throws AnalysisException {
     private void handleShowDb() throws AnalysisException {
         ShowDbStmt showDbStmt = (ShowDbStmt) stmt;
         List<List<String>> rows = Lists.newArrayList();
-        List<String> dbNames = ctx.getCatalog().getInternalDataSource().getClusterDbNames(ctx.getClusterName());
+        // external catalog have on cluster semantics
+        List<String> dbNames;
+        if (ctx.getDefaultCatalog().equals(InternalDataSource.INTERNAL_DS_NAME)) {

Review Comment:
   We can just use `ctx.getCurrentDataSource().getDbNames();` and ignore the `cluster` info.
   Because `cluster` feature is deprecated.



##########
fe/fe-core/src/main/java/org/apache/doris/analysis/FromClause.java:
##########
@@ -77,17 +72,10 @@ private void checkFromHiveTable(Analyzer analyzer) throws AnalysisException {
             }
 
             TableName tableName = tblRef.getName();

Review Comment:
   This method `checkFromHiveTable()` can be removed



##########
fe/fe-core/src/main/java/org/apache/doris/policy/Policy.java:
##########
@@ -84,11 +84,10 @@ public static Policy fromCreateStmt(CreatePolicyStmt stmt) throws AnalysisExcept
                 return storagePolicy;
             case ROW:
             default:
-                String curDb = stmt.getTableName().getDb();
-                if (curDb == null) {
-                    curDb = ConnectContext.get().getDatabase();
-                }
-                DatabaseIf db = Catalog.getCurrentCatalog().getCurrentDataSource().getDbOrAnalysisException(curDb);
+                // stmt must be analyzed.
+                DatabaseIf db = Catalog.getCurrentCatalog().getDataSourceMgr()
+                        .getCatalog(stmt.getTableName().getCtl())

Review Comment:
   The `getCatalog()` method may return null here if catalog does not exist.
   We can use `Optional` to do this.



##########
fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java:
##########
@@ -221,7 +221,9 @@ public TGetTablesResult getTableNames(TGetTablesParams params) throws TException
         } else {
             currentUser = UserIdentity.createAnalyzedUserIdentWithIp(params.user, params.user_ip);
         }
-        DatabaseIf<TableIf> db = Catalog.getCurrentCatalog().getCurrentDataSource().getDbNullable(params.db);
+        String catalog = Strings.isNullOrEmpty(params.catalog) ? InternalDataSource.INTERNAL_DS_NAME : params.catalog;
+        DatabaseIf<TableIf> db = Catalog.getCurrentCatalog().getDataSourceMgr()

Review Comment:
   `getCatalog` may return null



##########
fe/fe-core/src/main/java/org/apache/doris/qe/ShowExecutor.java:
##########
@@ -1005,8 +1013,9 @@ private void handleHelp() {
     private void handleShowLoad() throws AnalysisException {
         ShowLoadStmt showStmt = (ShowLoadStmt) stmt;
 
-        Catalog catalog = Catalog.getCurrentCatalog();
-        DatabaseIf db = catalog.getCurrentDataSource().getDbOrAnalysisException(showStmt.getDbName());
+        Util.prohibitExternalCatalog(ctx.getDefaultCatalog(), stmt.getClass().getSimpleName());
+        Catalog catalog = ctx.getCatalog();

Review Comment:
   Unused?



##########
fe/fe-core/src/main/java/org/apache/doris/analysis/FunctionCallExpr.java:
##########
@@ -936,7 +936,8 @@ public void analyzeImpl(Analyzer analyzer) throws AnalysisException {
                                 .checkDbPriv(ConnectContext.get(), dbName, PrivPredicate.SELECT)) {
                             ErrorReport.reportAnalysisException(ErrorCode.ERR_SPECIFIC_ACCESS_DENIED_ERROR, "SELECT");
                         }
-                        DatabaseIf db = Catalog.getCurrentCatalog().getCurrentDataSource().getDbNullable(dbName);
+                        // TODO(gaoxin): ExternalDatabase not implement udf yet.
+                        DatabaseIf db = Catalog.getCurrentCatalog().getInternalDataSource().getDbNullable(dbName);

Review Comment:
   Maybe we should modify `fnName` too, same as `TableName`



##########
fe/fe-core/src/main/java/org/apache/doris/httpv2/rest/MetaInfoAction.java:
##########
@@ -90,8 +90,9 @@ public Object getAllDatabases(
             return ResponseEntityBuilder.badRequest("Only support 'default_cluster' now");
         }
 
+        // TODO(gaoxin): Implement multi-catalog for http restful api.
         // 1. get all database with privilege
-        List<String> dbNames = Catalog.getCurrentCatalog().getCurrentDataSource().getDbNames();

Review Comment:
   we can use `NS_KEY`.
   but `NS_KEY`'s default value is `default_cluster`, we need to do some compatible things.
   For example, if `NS_KEY` is `default_cluster`, change it to `internal` catalog implicitly, otherwise, use it as catalog name.



##########
fe/fe-core/src/main/java/org/apache/doris/analysis/DescribeStmt.java:
##########
@@ -75,6 +79,13 @@ public class DescribeStmt extends ShowStmt {
                     .addColumn(new Column("Table", ScalarType.createVarchar(30)))
                     .build();
 
+    private static final ShowResultSetMetaData HMS_EXTERNAL_TABLE_META_DATA =

Review Comment:
   I think we should use columns defined in `IndexSchemaProcNode`.
   To be compatible with mysql style



-- 
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-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [doris] github-actions[bot] commented on pull request #10521: [feature-wip](multi-catalog) end to end to support multi-catalog

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #10521:
URL: https://github.com/apache/doris/pull/10521#issuecomment-1172129998

   PR approved by anyone and no changes requested.


-- 
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-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [doris] github-actions[bot] commented on pull request #10521: [feature-wip](multi-catalog) end to end to support multi-catalog

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #10521:
URL: https://github.com/apache/doris/pull/10521#issuecomment-1172129941

   PR approved by at least one committer and no changes requested.


-- 
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-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [doris] AshinGau commented on a diff in pull request #10521: [feature-wip](multi-catalog) end to end to support multi-catalog

Posted by GitBox <gi...@apache.org>.
AshinGau commented on code in PR #10521:
URL: https://github.com/apache/doris/pull/10521#discussion_r911739124


##########
fe/fe-core/src/main/java/org/apache/doris/qe/ShowExecutor.java:
##########
@@ -1005,8 +1013,9 @@ private void handleHelp() {
     private void handleShowLoad() throws AnalysisException {
         ShowLoadStmt showStmt = (ShowLoadStmt) stmt;
 
-        Catalog catalog = Catalog.getCurrentCatalog();
-        DatabaseIf db = catalog.getCurrentDataSource().getDbOrAnalysisException(showStmt.getDbName());
+        Util.prohibitExternalCatalog(ctx.getDefaultCatalog(), stmt.getClass().getSimpleName());
+        Catalog catalog = ctx.getCatalog();

Review Comment:
   `Catalog.getCurrentCatalog()` gets `Catalog` from thread local, and `ctx.getCatalog()` gets `Catalog` from its own. Using the self bound variable can check whether the`ConnectContext` is initialized correctly.



-- 
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-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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


[GitHub] [doris] AshinGau commented on a diff in pull request #10521: [feature-wip](multi-catalog) end to end to support multi-catalog

Posted by GitBox <gi...@apache.org>.
AshinGau commented on code in PR #10521:
URL: https://github.com/apache/doris/pull/10521#discussion_r911764266


##########
fe/fe-core/src/main/java/org/apache/doris/analysis/FunctionCallExpr.java:
##########
@@ -936,7 +936,8 @@ public void analyzeImpl(Analyzer analyzer) throws AnalysisException {
                                 .checkDbPriv(ConnectContext.get(), dbName, PrivPredicate.SELECT)) {
                             ErrorReport.reportAnalysisException(ErrorCode.ERR_SPECIFIC_ACCESS_DENIED_ERROR, "SELECT");
                         }
-                        DatabaseIf db = Catalog.getCurrentCatalog().getCurrentDataSource().getDbNullable(dbName);
+                        // TODO(gaoxin): ExternalDatabase not implement udf yet.
+                        DatabaseIf db = Catalog.getCurrentCatalog().getInternalDataSource().getDbNullable(dbName);

Review Comment:
   support later.



-- 
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-unsubscribe@doris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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