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-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
