github-actions[bot] commented on code in PR #67163:
URL: https://github.com/apache/doris/pull/67163#discussion_r3861791583
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergMetadataOps.java:
##########
@@ -249,7 +249,7 @@ private boolean performCreateDb(String dbName, boolean
ifNotExists, Map<String,
}
if (!properties.isEmpty() && dorisCatalog instanceof
IcebergExternalCatalog) {
String icebergCatalogType = ((IcebergExternalCatalog)
dorisCatalog).getIcebergCatalogType();
- if
(!IcebergExternalCatalog.ICEBERG_HMS.equals(icebergCatalogType)) {
+ if (!supportsDatabaseProperties(icebergCatalogType)) {
Review Comment:
[P1] Verify that REST actually stored the properties
Iceberg 1.10.1 explicitly permits REST implementations not to support
namespace properties: a successful `CreateNamespaceResponse` only includes
properties when the server stored them ([REST
contract](https://github.com/apache/iceberg/blob/apache-iceberg-1.10.1/open-api/rest-catalog-open-api.yaml#L4586-L4595)).
Its `RESTSessionCatalog.createNamespace` sends this map but then ignores that
response ([client
path](https://github.com/apache/iceberg/blob/apache-iceberg-1.10.1/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java#L540-L556)),
and this method returns success without reloading the namespace. On such a
conforming server, `CREATE DATABASE ... PROPERTIES ('location'=...)` can
therefore succeed while the requested location (or owner/custom metadata) is
discarded, so later tables use the server's default location. The new
regression only proves one REST implementation preserves `location`. Please
require an explicit server capability/opt-in, or reload and verify t
he requested metadata after creation with truthful cache/edit-log and
partial-create handling, before treating every REST catalog as supported.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonMetadataOps.java:
##########
@@ -109,7 +109,7 @@ private boolean performCreateDb(String dbName, boolean
ifNotExists, Map<String,
if (!properties.isEmpty() && dorisCatalog instanceof
PaimonExternalCatalog) {
String catalogType = ((PaimonExternalCatalog)
dorisCatalog).getCatalogType();
- if (!PaimonExternalCatalog.PAIMON_HMS.equals(catalogType)) {
+ if (!supportsDatabaseProperties(catalogType)) {
Review Comment:
[P2] Make SHOW CREATE reproduce the accepted metadata
These newly accepted JDBC/REST/DLF properties are stored by the Paimon
catalog, but Doris never loads them into the refreshed
`PaimonExternalDatabase`: `afterCreateDb` only resets names, the external
database starts with an empty `DatabaseProperty`, and `SHOW CREATE DATABASE`
consequently emits a bare statement. For example, a successful create with a
custom `location` or `owner` cannot be reproduced even though Paimon 1.4.2
returns those options from `Catalog.getDatabase`. The sibling Iceberg path has
the same gap for newly accepted `owner`/custom metadata because its SHOW branch
renders only `location`. Please make SHOW read a create-compatible projection
of the current remote metadata for both paths (filtering
server-managed/default/audit fields), and add round-trip assertions; the
current JDBC regressions only verify that the database name 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: [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]
