morningman commented on code in PR #19242:
URL: https://github.com/apache/doris/pull/19242#discussion_r1186269770
##########
fe/fe-core/src/main/java/org/apache/doris/qe/ShowExecutor.java:
##########
@@ -1567,10 +1570,42 @@ private void handleShowData() throws AnalysisException {
private void handleShowPartitions() throws AnalysisException {
ShowPartitionsStmt showStmt = (ShowPartitionsStmt) stmt;
- ProcNodeInterface procNodeI = showStmt.getNode();
- Preconditions.checkNotNull(procNodeI);
- List<List<String>> rows = ((PartitionsProcDir)
procNodeI).fetchResultByFilter(showStmt.getFilterMap(),
- showStmt.getOrderByPairs(),
showStmt.getLimitElement()).getRows();
+ if (showStmt.getCatalog().isInternalCatalog()) {
+ ProcNodeInterface procNodeI = showStmt.getNode();
+ Preconditions.checkNotNull(procNodeI);
+ List<List<String>> rows = ((PartitionsProcDir)
procNodeI).fetchResultByFilter(showStmt.getFilterMap(),
+ showStmt.getOrderByPairs(),
showStmt.getLimitElement()).getRows();
+ resultSet = new ShowResultSet(showStmt.getMetaData(), rows);
+ } else {
+ handleShowHMSTablePartitions(showStmt);
+ }
+ }
+
+ private void handleShowHMSTablePartitions(ShowPartitionsStmt showStmt)
throws AnalysisException {
+ CatalogIf catalog = showStmt.getCatalog();
+ DatabaseIf database =
catalog.getDbOrAnalysisException(showStmt.getTableName().getDb());
+ HMSExternalTable hmsTable = (HMSExternalTable)
database.getTableNullable(showStmt.getTableName().getTbl());
+ List<Type> partitionColumnTypes = hmsTable.getPartitionColumnTypes();
+
+ List<List<String>> rows = new ArrayList<>();
+ HiveMetaStoreCache cache = Env.getCurrentEnv().getExtMetaCacheMgr()
+ .getMetaStoreCache((HMSExternalCatalog) catalog);
+ if (!partitionColumnTypes.isEmpty()) {
+ HiveMetaStoreCache.HivePartitionValues hivePartitionValues =
+ cache.getPartitionValues(hmsTable.getDbName(),
hmsTable.getName(), partitionColumnTypes);
Review Comment:
If there are too many partitions, this operation will refresh the entire
cache and may impact other query.
I suggest that for `show partitions` operation, not using cache, but to get
partition directly from hms.
And we also need to consider if partition num is too large, eg, 10
thousands, do we need to limit the num of returned rows?
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/ShowPartitionsStmt.java:
##########
@@ -137,8 +151,14 @@ public void analyze(Analyzer analyzer) throws
UserException {
public void analyzeImpl(Analyzer analyzer) throws UserException {
super.analyze(analyzer);
tableName.analyze(analyzer);
- // disallow external catalog
- Util.prohibitExternalCatalog(tableName.getCtl(),
this.getClass().getSimpleName());
+ // disallow unsupported catalog
+ String catalogName = tableName.getCtl();
+ catalog = Env.getCurrentEnv().getCatalogMgr().getCatalog(catalogName);
+ String catalogType = catalog.getType();
+ if (!Strings.isNullOrEmpty(catalogName) &&
!(catalog.isInternalCatalog() || catalogType.equals("hms"))) {
+ throw new AnalysisException(String.format("Catalog of type '%s' is
not allowed in '%s'", catalogType,
+ this.getClass().getSimpleName()));
Review Comment:
Why using `this.getClass().getSimpleName()`?
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/ShowPartitionsStmt.java:
##########
@@ -137,8 +151,14 @@ public void analyze(Analyzer analyzer) throws
UserException {
public void analyzeImpl(Analyzer analyzer) throws UserException {
super.analyze(analyzer);
tableName.analyze(analyzer);
- // disallow external catalog
- Util.prohibitExternalCatalog(tableName.getCtl(),
this.getClass().getSimpleName());
+ // disallow unsupported catalog
+ String catalogName = tableName.getCtl();
+ catalog = Env.getCurrentEnv().getCatalogMgr().getCatalog(catalogName);
+ String catalogType = catalog.getType();
+ if (!Strings.isNullOrEmpty(catalogName) &&
!(catalog.isInternalCatalog() || catalogType.equals("hms"))) {
Review Comment:
No need to check `!Strings.isNullOrEmpty(catalogName)`, because after
`tableName.analyze(analyzer)`, the catalogName will be filled.
And you can use `catalog instanceof xxx` to check catalog's type, instead of
using string compare.
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/ShowPartitionsStmt.java:
##########
@@ -137,8 +151,14 @@ public void analyze(Analyzer analyzer) throws
UserException {
public void analyzeImpl(Analyzer analyzer) throws UserException {
super.analyze(analyzer);
tableName.analyze(analyzer);
- // disallow external catalog
- Util.prohibitExternalCatalog(tableName.getCtl(),
this.getClass().getSimpleName());
+ // disallow unsupported catalog
+ String catalogName = tableName.getCtl();
+ catalog = Env.getCurrentEnv().getCatalogMgr().getCatalog(catalogName);
Review Comment:
catalog may be null here if `catalogName` does not exists.
--
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
