diqiu50 commented on code in PR #4900:
URL: https://github.com/apache/gravitino/pull/4900#discussion_r1759700640
##########
docs/lakehouse-iceberg-catalog.md:
##########
@@ -36,12 +36,12 @@ Builds with Apache Iceberg `1.5.2`. The Apache Iceberg
table format version is `
### Catalog properties
-| Property name | Description
| Default value
| Required | Since Version
|
-|----------------------------------------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|------------------------|-------------------------------------------------------------|---------------|
-| `catalog-backend` | Catalog backend of
Gravitino Iceberg catalog. Supports `hive` or `jdbc` or `rest`.
| (none)
| Yes | 0.2.0
|
-| `uri` | The URI configuration
of the Iceberg catalog. `thrift://127.0.0.1:9083` or
`jdbc:postgresql://127.0.0.1:5432/db_name` or
`jdbc:mysql://127.0.0.1:3306/metastore_db` or `http://127.0.0.1:9001`.
| (none) | Yes
| 0.2.0 |
-| `warehouse` | Warehouse directory of
catalog. `file:///user/hive/warehouse-hive/` for local fs or
`hdfs://namespace/hdfs/path` for HDFS.
|
(none) | Yes
| 0.2.0 |
-| `catalog-backend-name` | The catalog name passed
to underlying Iceberg catalog backend. Catalog name in JDBC backend is used to
isolate namespace and tables.
| Gravitino catalog
name | No | 0.5.2
|
+| Property name | Description
| Default value
| Required |
Since Version |
+|----------------------------------------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|-----------------------------------|-------------------------------------------------------------|---------------|
+| `catalog-backend` | Catalog backend of
Gravitino Iceberg catalog. Supports `hive` or `jdbc` or `rest`.
| (none)
| Yes
| 0.2.0 |
+| `uri` | The URI configuration
of the Iceberg catalog. `thrift://127.0.0.1:9083` or
`jdbc:postgresql://127.0.0.1:5432/db_name` or
`jdbc:mysql://127.0.0.1:3306/metastore_db` or `http://127.0.0.1:9001`.
| (none) |
Yes | 0.2.0 |
+| `warehouse` | Warehouse directory of
catalog. `file:///user/hive/warehouse-hive/` for local fs or
`hdfs://namespace/hdfs/path` for HDFS.
|
(none) | Yes
| 0.2.0 |
+| `catalog-backend-name` | The catalog name passed
to underlying Iceberg catalog backend. Catalog name in JDBC backend is used to
isolate namespace and tables.
| `catalog-backend`
property value. | No |
0.5.2 |
Review Comment:
The default value is "**catalog-backend**" or "**`catalog-backend`
property value**".
##########
trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/iceberg/IcebergConnectorAdapter.java:
##########
@@ -46,7 +46,6 @@ public IcebergConnectorAdapter() {
@Override
public Map<String, String> buildInternalConnectorConfig(GravitinoCatalog
catalog)
throws Exception {
- catalog.getProperties().put("catalog-name", catalog.getName());
Review Comment:
```java
/** Help Apache Gravitino connector access CatalogMetadata from Gravitino
client. */
public class GravitinoCatalog {
private static ObjectMapper objectMapper = new ObjectMapper();
private final String metalake;
private final String provider;
private final String name;
private final Map<String, String> properties;
private final long lastModifiedTime;
static {
objectMapper =
JsonMapper.builder()
.disable(MapperFeature.AUTO_DETECT_CREATORS)
.disable(MapperFeature.AUTO_DETECT_FIELDS)
.disable(MapperFeature.AUTO_DETECT_SETTERS)
.disable(MapperFeature.AUTO_DETECT_GETTERS)
.build();
}
public GravitinoCatalog(String metalake, Catalog catalog) {
this.metalake = metalake;
this.provider = catalog.provider();
this.name = catalog.name();
this.properties = new HashMap<>(catalog.properties());
Instant time =
catalog.auditInfo().lastModifiedTime() == null
? catalog.auditInfo().createTime()
: catalog.auditInfo().lastModifiedTime();
lastModifiedTime = time.toEpochMilli();
}
```
After we remove this code, The properties map should not copy from the
original catalog properties.
##########
catalogs/catalog-common/src/main/java/org/apache/gravitino/catalog/lakehouse/iceberg/IcebergPropertiesUtils.java:
##########
@@ -74,4 +76,22 @@ public static Map<String, String> toIcebergCatalogProperties(
});
return icebergProperties;
}
+
+ /**
+ * Get catalog backend name from Gravitino catalog properties.
+ *
+ * @param catalogProperties a map of Gravitino catalog properties.
+ * @return catalog backend name.
+ */
+ public static String getCatalogBackendName(Map<String, String>
catalogProperties) {
+ String backendName =
catalogProperties.get(IcebergConstants.CATALOG_BACKEND_NAME);
+ if (backendName != null) {
+ return backendName;
+ }
+
+ String catalogBackend =
catalogProperties.get(IcebergConstants.CATALOG_BACKEND);
+ return Optional.ofNullable(catalogBackend)
+ .map(s -> s.toLowerCase(Locale.ROOT))
+ .orElse("memory");
Review Comment:
What's the meaning of the default value `memory"?
--
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]
