coolderli commented on a change in pull request #2799:
URL: https://github.com/apache/iceberg/pull/2799#discussion_r667590594



##########
File path: flink/src/main/java/org/apache/iceberg/flink/CatalogLoader.java
##########
@@ -51,7 +51,9 @@ static CatalogLoader hadoop(String name, Configuration 
hadoopConf, Map<String, S
   }
 
   static CatalogLoader hive(String name, Configuration hadoopConf, Map<String, 
String> properties) {
-    return new HiveCatalogLoader(name, hadoopConf, properties);
+    String hiveConfDir = properties.get(FlinkCatalogFactory.HIVE_CONF_DIR);
+    Configuration newHadoopConf = 
FlinkCatalogFactory.mergeHiveConf(hadoopConf, hiveConfDir);

Review comment:
       Yes, it can be loaded into `HiveCatalog` successfully. But there are 
some other configurations (For example 
`hadoop.security.authentication`,`hive.metastore.kerberos.principal`) that 
could not be load by properties. Of course, this can be passed in through the 
`hadoopConf` . However, users must always pay attention to the consistency of 
code configuration and cluster environment(the configuration of hive-site.xml). 
It is true that users can pass the hive-site.xml to `hadoopConf` outside this 
method, but this needs to be consistent with hive-site.xml in hivecatalog.




-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to