[GitHub] [incubator-doris] qidaye commented on a diff in pull request #9658: Refactor part of Iceberg code to support more Iceberg Catalogs

[GitBox <gi...@apache.org>]

qidaye commented on code in PR #9658:
URL: https://github.com/apache/incubator-doris/pull/9658#discussion_r895050605


##########
docs/en/ecosystem/external-table/iceberg-of-doris.md:
##########
@@ -135,10 +167,14 @@ You can also create an Iceberg table by explicitly specifying the column definit
     - The order of the columns needs to be consistent with the Iceberg table
 - ENGINE needs to be specified as ICEBERG
 - PROPERTIES property.
-    - `iceberg.hive.metastore.uris`: Hive Metastore service address
     - `iceberg.database`: the name of the database to which Iceberg is mounted
     - `iceberg.table`: the name of the table to which Iceberg is mounted, not required when mounting Iceberg database.
-    - `iceberg.catalog.type`: the catalog method used in Iceberg, the default is `HIVE_CATALOG`, currently only this method is supported, more Iceberg catalog access methods will be supported in the future.
+    - `iceberg.catalog.type`: the catalog type used in Iceberg, the default is `HIVE` (simple name of `org.apache.iceberg.hive.HiveCatalog`). Iceberg built-in supports two catalogs with a simple name: `HIVE` and `HADOOP`. You should use the full class name for other implementations.

Review Comment:
   How about add more built-in catalogs, such as JDBC, Glue catalog, for user convenience ?
   
   User only need set full class name when using a custom catalog.



##########
fe/fe-core/src/main/java/org/apache/doris/catalog/IcebergProperty.java:
##########
@@ -17,33 +17,73 @@
 
 package org.apache.doris.catalog;
 
+import com.google.common.base.Joiner;
+import com.google.common.base.MoreObjects;
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Maps;
+
+import java.util.Locale;
 import java.util.Map;
+import java.util.Objects;
+
+import static org.apache.iceberg.CatalogUtil.ICEBERG_CATALOG_HADOOP;
+import static org.apache.iceberg.CatalogUtil.ICEBERG_CATALOG_HIVE;

Review Comment:
   Do not import static member.



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