github-actions[bot] commented on code in PR #67163:
URL: https://github.com/apache/doris/pull/67163#discussion_r3877673624
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/IcebergMetadataOps.java:
##########
@@ -258,6 +258,12 @@ private boolean performCreateDb(String dbName, boolean
ifNotExists, Map<String,
return false;
}
+ private boolean supportsDatabaseProperties(String catalogType) {
+ return IcebergExternalCatalog.ICEBERG_HMS.equals(catalogType)
+ || IcebergExternalCatalog.ICEBERG_JDBC.equals(catalogType)
Review Comment:
[P1] Reject JDBC `location` until table placement honors it
This makes Iceberg JDBC accept every database-property map, but the pinned
`JdbcCatalog` only persists `location`; its [table
default](https://github.com/apache/iceberg/blob/apache-iceberg-1.10.1/core/src/main/java/org/apache/iceberg/jdbc/JdbcCatalog.java#L282-L283)
still builds from the catalog warehouse without reading the stored namespace
metadata. Thus `CREATE DATABASE db PROPERTIES ('location'='s3://isolated/db')`
succeeds, yet `CREATE TABLE db.t` writes under `<warehouse>/db/t`. Paimon 1.4.2
JDBC has the same mismatch: it stores the supplied database location but
inherits
[`AbstractCatalog.getTableLocation`](https://github.com/apache/paimon/blob/release-1.4.2/paimon-core/src/main/java/org/apache/paimon/catalog/AbstractCatalog.java#L667-L699),
which always uses `<warehouse>/<db>.db/<table>`. This is distinct from the
SHOW CREATE issue because the metadata is durable; the table-placement path
ignores it. Please make support key-specific (reject `location` for these JDBC
types)
or use the stored database location, and add JDBC regressions that assert the
created table path.
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/paimon/PaimonMetadataOps.java:
##########
@@ -119,6 +119,13 @@ private boolean performCreateDb(String dbName, boolean
ifNotExists, Map<String,
return false;
}
+ private boolean supportsDatabaseProperties(String catalogType) {
+ return PaimonExternalCatalog.PAIMON_HMS.equals(catalogType)
+ || PaimonExternalCatalog.PAIMON_JDBC.equals(catalogType)
+ || PaimonExternalCatalog.PAIMON_REST.equals(catalogType)
Review Comment:
[P1] Verify REST used the requested database location
Paimon 1.4.2 posts `location` only inside the arbitrary options map; on
reload its client [overwrites that option with the response's separate location
field](https://github.com/apache/paimon/blob/release-1.4.2/paimon-core/src/main/java/org/apache/paimon/rest/RESTCatalog.java#L145-L171).
Its official REST server shows the mismatch concretely: it [creates the
backing database without the request
options](https://github.com/apache/paimon/blob/release-1.4.2/paimon-core/src/test/java/org/apache/paimon/rest/RESTCatalogServer.java#L1271-L1281),
stores those options separately, and later [creates tables through that
backing
catalog](https://github.com/apache/paimon/blob/release-1.4.2/paimon-core/src/test/java/org/apache/paimon/rest/RESTCatalogServer.java#L1457-L1481).
Doris returns success without reloading the database, so `CREATE DATABASE db
PROPERTIES ('location'='s3://isolated/db')` can succeed while `CREATE TABLE
db.t` is placed under the server default. This is separate from SHOW C
REATE: rendering the server's actual location does not honor the requested
one. Please reject `location` for REST unless supported, or reload and verify
the authoritative location before cache/edit-log success, with a REST
table-path regression.
--
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]
