morningman commented on code in PR #41659:
URL: https://github.com/apache/doris/pull/41659#discussion_r1796396660
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/hive/HMSExternalCatalog.java:
##########
@@ -85,6 +88,8 @@ public class HMSExternalCatalog extends ExternalCatalog {
private int hmsEventsBatchSizePerRpc = -1;
private boolean enableHmsEventsIncrementalSync = false;
+ //for "type" = "hms" , but is iceberg table.
+ HiveCatalog icebergHiveCatalog;
Review Comment:
```suggestion
private HiveCatalog icebergHiveCatalog;
```
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalTable.java:
##########
@@ -203,14 +208,15 @@ public long getRowCount() {
@Override
public long getCachedRowCount() {
- // Return -1 if makeSureInitialized throw exception.
- // For example, init hive table may throw NotSupportedException.
- try {
- makeSureInitialized();
- } catch (Exception e) {
- LOG.warn("Failed to initialize table {}.{}.{}", catalog.getName(),
dbName, name, e);
- return -1;
+ // Return -2 if uninitialized.
+ // Before this, for uninitialized tables, we would call
makeSureInitialized(), just like the implementation of
+ // ExternalTable.getRowCount(), but this is not very meaningful and
time-consuming.
+ // The getCachedRowCount() function is only used when `show table` and
querying `information_schema.tables`.
+ if (!checkInitialized()) {
+ return -2;
Review Comment:
Use -1
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/hive/HMSExternalCatalog.java:
##########
@@ -331,6 +336,13 @@ public boolean isEnableHmsEventsIncrementalSync() {
return enableHmsEventsIncrementalSync;
}
+ public Catalog getIcebergHiveCatalog() {
+ if (icebergHiveCatalog == null) {
Review Comment:
This method may be called concurrently
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergMetadataCache.java:
##########
@@ -177,29 +170,6 @@ public void invalidateDbCache(long catalogId, String
dbName) {
});
}
- private Catalog createIcebergHiveCatalog(String uri, Map<String, String>
hdfsConf, Map<String, String> props) {
- // set hdfs configure
- Configuration conf = DFSFileSystem.getHdfsConf(
-
hdfsConf.getOrDefault(DFSFileSystem.PROP_ALLOW_FALLBACK_TO_SIMPLE_AUTH,
"").isEmpty());
- for (Map.Entry<String, String> entry : hdfsConf.entrySet()) {
- conf.set(entry.getKey(), entry.getValue());
- }
- HiveCatalog hiveCatalog = new HiveCatalog();
- hiveCatalog.setConf(conf);
-
- if (props.containsKey(HMSExternalCatalog.BIND_BROKER_NAME)) {
Review Comment:
This part is missing?
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalTable.java:
##########
@@ -114,6 +114,11 @@ public boolean isView() {
return false;
}
+ public boolean checkInitialized() {
Review Comment:
No need this method, we can use check `objectCreated`
--
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
