This is an automated email from the ASF dual-hosted git repository.
morningman pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 3f4a85a5cfc [fix](catalog) Allow Iceberg JDBC and Glue database
properties and reject Paimon path changes (#69228)
3f4a85a5cfc is described below
commit 3f4a85a5cfcc2dd250373171a97217058d520642
Author: Mingyu Chen (Rayner) <[email protected]>
AuthorDate: Mon Oct 19 07:55:43 2026 +0100
[fix](catalog) Allow Iceberg JDBC and Glue database properties and reject
Paimon path changes (#69228)
### What problem does this PR solve?
Issue Number: None
Related PR: #67163
Problem Summary:
**In short.** Ports the rest of branch-4.1 #67163 to master. Iceberg
JDBC and Glue catalogs accept `CREATE DATABASE ... PROPERTIES (...)`,
and Glue also accepts `location`. A Paimon `ALTER TABLE ... SET` that
names `path` is rejected before anything is committed.
**Background.** #67163 aligned database properties with what each
catalog type can store and use, and rejected the Paimon `path` option in
`ALTER TABLE ... SET`. Master has the Paimon database part since #67395
(`PaimonConnectorMetadata.createDatabase`: HMS, JDBC, REST and DLF store
properties, only HMS and DLF use `location`). The Iceberg part was not
ported, and Paimon `ALTER TABLE ... SET` came to master later, with
#69086, without the `path` check.
**The problem, and what it cost.**
- Iceberg accepts database properties only for HMS catalogs. On a JDBC
or Glue catalog, which store namespace properties, `CREATE DATABASE db
PROPERTIES ('owner' = 'doris')` fails with `Not supported: create
database with properties for iceberg catalog type: jdbc`.
- `ALTER TABLE paimon_t SET ('path' = '...')` commits a new schema
version that stores the new path. Paimon leaves that check to the engine
(`SchemaManager.checkAlterTablePath`, which nothing in paimon-core
calls). The table does not move: its files stay where they are, and the
catalog keeps opening it there, so the stored path is wrong metadata,
shown by `$options`.
**How this PR fixes it.**
- `IcebergConnectorMetadata.createDatabase` checks the catalog type the
way the Paimon connector does:
| Iceberg catalog type | Database properties | `location` |
|---|---|---|
| HMS, Glue | Accepted, forwarded unchanged | Accepted |
| JDBC | Accepted, forwarded unchanged | Rejected: the catalog would
store it but not place new tables under it |
| Hadoop, REST, DLF, S3 Tables | Rejected, as before (a REST server may
ignore namespace properties) | Rejected |
- `PaimonConnectorMetadata.setTableProperties` runs every key through
`SchemaManager.checkAlterTablePath` before the remote alter. A `path`
entry, in any case, fails with `Change path is not supported yet.`, and
the other entries of the statement are not committed either.
**Results.** On a local cluster with the Iceberg docker environment, and
PostgreSQL 14 and MySQL 5.7 for the Iceberg JDBC catalog:
| Statement | Before | After |
|---|---|---|
| Iceberg JDBC (PostgreSQL): `CREATE DATABASE db PROPERTIES ('owner' =
'doris', 'team' = 'lakehouse')` | `Not supported: create database with
properties for iceberg catalog type: jdbc` | Created;
`iceberg_namespace_properties` has `owner` and `team` |
| Iceberg JDBC: `CREATE DATABASE db PROPERTIES ('location' =
's3://...')` | Same error | `Not supported: database property 'location'
for iceberg catalog type: jdbc because it does not determine the default
table location` |
| Paimon: `ALTER TABLE t SET ('PATH' =
's3://warehouse/relocated_table')` | New schema version whose options
store the new path; data stays in place | `Change path is not supported
yet.`; schema version and `$options` unchanged |
The Paimon "before" row is what paimon-core does with that `SetOption`
on a local `FileSystemCatalog`: schema 1 stores `path = <new location>`,
while the files and the location the catalog opens stay where they were.
Co-authored-by: Socrates <[email protected]>
---
.../iceberg/IcebergConnectorMetadata.java | 29 +++++++---
.../iceberg/IcebergConnectorMetadataDdlTest.java | 62 +++++++++++++++++++++-
.../connector/paimon/PaimonConnectorMetadata.java | 15 +++++-
...PaimonConnectorMetadataTablePropertiesTest.java | 49 +++++++++++++++++
.../paimon/test_paimon_alter_properties.out | 6 +++
.../iceberg/test_iceberg_jdbc_catalog.groovy | 12 ++++-
.../paimon/test_paimon_alter_properties.groovy | 12 +++++
7 files changed, 174 insertions(+), 11 deletions(-)
diff --git
a/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnectorMetadata.java
b/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnectorMetadata.java
index 0516fc0b15a..a1259061585 100644
---
a/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnectorMetadata.java
+++
b/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnectorMetadata.java
@@ -939,16 +939,22 @@ public class IcebergConnectorMetadata implements
ConnectorMetadata {
// ========== DDL writes (B1): create/drop database + table ==========
/**
- * Creates an iceberg namespace, mirroring legacy {@code
IcebergMetadataOps.performCreateDb}. Namespace
- * properties are only honored by an HMS catalog; for every other flavor a
non-empty property map fails
- * loud (legacy parity) — the gate is a pure local check run BEFORE the
auth context, like paimon.
- * Existence / IF NOT EXISTS is resolved upstream by {@code
PluginDrivenExternalCatalog.createDb}.
+ * Creates an iceberg namespace, mirroring legacy {@code
IcebergMetadataOps.performCreateDb}. HMS, JDBC
+ * and Glue catalogs store namespace properties; only HMS and Glue place
new tables under a namespace
+ * {@code location}. Hadoop, REST, DLF and S3 Tables catalogs reject a
non-empty property map (a REST
+ * server may ignore namespace properties). The gate is a pure local check
run BEFORE the auth context,
+ * like paimon. Existence / IF NOT EXISTS is resolved upstream by {@code
PluginDrivenExternalCatalog.createDb}.
*/
@Override
public void createDatabase(ConnectorSession session, String dbName,
Map<String, String> properties) {
- if (!properties.isEmpty() && !isHmsCatalog()) {
+ String catalogType = catalogType();
+ if (!properties.isEmpty() && !supportsDatabaseProperties(catalogType))
{
throw new DorisConnectorException(
- "Not supported: create database with properties for
iceberg catalog type: " + catalogType());
+ "Not supported: create database with properties for
iceberg catalog type: " + catalogType);
+ }
+ if (properties.containsKey("location") &&
!supportsDatabaseLocation(catalogType)) {
+ throw new DorisConnectorException("Not supported: database
property 'location' for iceberg catalog type: "
+ + catalogType + " because it does not determine the
default table location");
}
try {
executeAuthenticated(() -> {
@@ -961,6 +967,17 @@ public class IcebergConnectorMetadata implements
ConnectorMetadata {
}
}
+ private static boolean supportsDatabaseProperties(String catalogType) {
+ return IcebergCatalogProperties.TYPE_HMS.equals(catalogType)
+ || IcebergCatalogProperties.TYPE_JDBC.equals(catalogType)
+ || IcebergCatalogProperties.TYPE_GLUE.equals(catalogType);
+ }
+
+ private static boolean supportsDatabaseLocation(String catalogType) {
+ return IcebergCatalogProperties.TYPE_HMS.equals(catalogType)
+ || IcebergCatalogProperties.TYPE_GLUE.equals(catalogType);
+ }
+
/**
* Drops an iceberg namespace, mirroring legacy {@code
IcebergMetadataOps.performDropDb}. With
* {@code force} the contained tables are dropped (purged) first so a
non-empty namespace can be removed;
diff --git
a/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergConnectorMetadataDdlTest.java
b/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergConnectorMetadataDdlTest.java
index ff9dc79bc70..a767e72437f 100644
---
a/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergConnectorMetadataDdlTest.java
+++
b/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergConnectorMetadataDdlTest.java
@@ -42,7 +42,7 @@ import java.util.Optional;
/**
* Behavior tests for the B1 DDL overrides on {@link IcebergConnectorMetadata}
— driven entirely through the
* {@link RecordingIcebergCatalogOps} seam + {@link RecordingConnectorContext}
(no live catalog, no Mockito).
- * Asserts: every remote op runs INSIDE the auth context, the HMS-only
properties gate, the force-drop
+ * Asserts: every remote op runs INSIDE the auth context, the per-catalog-type
properties gate, the force-drop
* cascade, and that the managed-location cleanup hook is invoked (HMS only)
with the location captured
* BEFORE the drop.
*/
@@ -75,7 +75,7 @@ public class IcebergConnectorMetadataDdlTest {
}
@Test
- public void testCreateDatabaseNonHmsWithPropertiesFailsLoud() {
+ public void testCreateDatabaseRestWithPropertiesFailsLoud() {
RecordingIcebergCatalogOps ops = new RecordingIcebergCatalogOps();
RecordingConnectorContext ctx = new RecordingConnectorContext();
IcebergConnectorMetadata md = metadata(ops, ctx,
IcebergCatalogProperties.TYPE_REST);
@@ -87,6 +87,64 @@ public class IcebergConnectorMetadataDdlTest {
Assertions.assertEquals(0, ctx.authCount);
}
+ @Test
+ public void testCreateDatabaseWithPropertiesForSupportedCatalogs() {
+ // HMS, JDBC and Glue store namespace properties; the map is forwarded
unchanged.
+ Map<String, String> dbProps = Collections.singletonMap("owner",
"doris");
+ for (String catalogType :
Arrays.asList(IcebergCatalogProperties.TYPE_HMS,
+ IcebergCatalogProperties.TYPE_JDBC,
IcebergCatalogProperties.TYPE_GLUE)) {
+ RecordingIcebergCatalogOps ops = new RecordingIcebergCatalogOps();
+ RecordingConnectorContext ctx = new RecordingConnectorContext();
+ metadata(ops, ctx, catalogType).createDatabase(null, "db1",
dbProps);
+ Assertions.assertEquals("db1", ops.lastCreateDb, catalogType);
+ Assertions.assertEquals(dbProps, ops.lastCreateDbProps,
catalogType);
+ Assertions.assertEquals(1, ctx.authCount, catalogType);
+ }
+ }
+
+ @Test
+ public void testCreateDatabaseWithLocationForSupportedCatalogs() {
+ // Only HMS and Glue place new tables under the namespace location.
+ Map<String, String> dbProps = Collections.singletonMap("location",
"s3://wh/db1");
+ for (String catalogType :
Arrays.asList(IcebergCatalogProperties.TYPE_HMS,
+ IcebergCatalogProperties.TYPE_GLUE)) {
+ RecordingIcebergCatalogOps ops = new RecordingIcebergCatalogOps();
+ RecordingConnectorContext ctx = new RecordingConnectorContext();
+ metadata(ops, ctx, catalogType).createDatabase(null, "db1",
dbProps);
+ Assertions.assertEquals(dbProps, ops.lastCreateDbProps,
catalogType);
+ }
+ }
+
+ @Test
+ public void testCreateDatabaseWithLocationForJdbcCatalogIsRejected() {
+ // A JDBC catalog would store the location without placing later
tables under it.
+ RecordingIcebergCatalogOps ops = new RecordingIcebergCatalogOps();
+ RecordingConnectorContext ctx = new RecordingConnectorContext();
+ IcebergConnectorMetadata md = metadata(ops, ctx,
IcebergCatalogProperties.TYPE_JDBC);
+ DorisConnectorException ex =
Assertions.assertThrows(DorisConnectorException.class,
+ () -> md.createDatabase(null, "db1",
Collections.singletonMap("location", "s3://wh/db1")));
+ Assertions.assertTrue(ex.getMessage().contains("database property
'location' for iceberg catalog type: jdbc"),
+ ex.getMessage());
+ Assertions.assertTrue(ops.log.isEmpty(), ops.log.toString());
+ Assertions.assertEquals(0, ctx.authCount);
+ }
+
+ @Test
+ public void testCreateDatabaseWithPropertiesForUnsupportedCatalogs() {
+ for (String catalogType :
Arrays.asList(IcebergCatalogProperties.TYPE_HADOOP,
+ IcebergCatalogProperties.TYPE_REST,
IcebergCatalogProperties.TYPE_DLF,
+ IcebergCatalogProperties.TYPE_S3_TABLES)) {
+ RecordingIcebergCatalogOps ops = new RecordingIcebergCatalogOps();
+ RecordingConnectorContext ctx = new RecordingConnectorContext();
+ IcebergConnectorMetadata md = metadata(ops, ctx, catalogType);
+ DorisConnectorException ex =
Assertions.assertThrows(DorisConnectorException.class,
+ () -> md.createDatabase(null, "db1",
Collections.singletonMap("owner", "doris")));
+ Assertions.assertTrue(ex.getMessage().contains("iceberg catalog
type: " + catalogType), ex.getMessage());
+ Assertions.assertTrue(ops.log.isEmpty(), catalogType + " " +
ops.log);
+ Assertions.assertEquals(0, ctx.authCount, catalogType);
+ }
+ }
+
@Test
public void testCreateDatabaseNonHmsEmptyPropertiesSucceeds() {
RecordingIcebergCatalogOps ops = new RecordingIcebergCatalogOps();
diff --git
a/fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonConnectorMetadata.java
b/fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonConnectorMetadata.java
index 967d60df63a..b6dd5993eb2 100644
---
a/fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonConnectorMetadata.java
+++
b/fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonConnectorMetadata.java
@@ -49,6 +49,7 @@ import org.apache.paimon.catalog.Identifier;
import org.apache.paimon.partition.Partition;
import org.apache.paimon.schema.Schema;
import org.apache.paimon.schema.SchemaChange;
+import org.apache.paimon.schema.SchemaManager;
import org.apache.paimon.table.DataTable;
import org.apache.paimon.table.FileStoreTable;
import org.apache.paimon.table.Table;
@@ -1198,13 +1199,25 @@ public class PaimonConnectorMetadata implements
ConnectorMetadata {
* {@link SchemaChange#setOption} in ONE {@code alterTable} call: paimon
validates the whole list against
* the new schema before committing it, so a rejected entry keeps the
valid ones from being written too,
* and the table gets one new schema version rather than one per option.
+ *
+ * <p>{@code path} is rejected before the remote alter: paimon would
commit it as an ordinary option, but
+ * changing it does not relocate the table. Paimon leaves that check to
the engine
+ * ({@link SchemaManager#checkAlterTablePath}).
*/
@Override
public void setTableProperties(ConnectorSession session,
ConnectorTableHandle handle,
Map<String, String> properties) {
+ PaimonTableHandle paimonHandle = (PaimonTableHandle) handle;
+ try {
+ properties.keySet().forEach(SchemaManager::checkAlterTablePath);
+ } catch (UnsupportedOperationException e) {
+ throw new DorisConnectorException("Failed to set properties for
Paimon table "
+ + Identifier.create(paimonHandle.getDatabaseName(),
paimonHandle.getTableName())
+ + ": " + e.getMessage(), e);
+ }
List<SchemaChange> changes = new ArrayList<>(properties.size());
properties.forEach((key, value) ->
changes.add(SchemaChange.setOption(key, value)));
- alterTable((PaimonTableHandle) handle, changes, "set properties");
+ alterTable(paimonHandle, changes, "set properties");
}
// ==================== DDL: Column evolution ====================
diff --git
a/fe/fe-connector/fe-connector-paimon/src/test/java/org/apache/doris/connector/paimon/PaimonConnectorMetadataTablePropertiesTest.java
b/fe/fe-connector/fe-connector-paimon/src/test/java/org/apache/doris/connector/paimon/PaimonConnectorMetadataTablePropertiesTest.java
index c745ca9ee9a..83115772cb0 100644
---
a/fe/fe-connector/fe-connector-paimon/src/test/java/org/apache/doris/connector/paimon/PaimonConnectorMetadataTablePropertiesTest.java
+++
b/fe/fe-connector/fe-connector-paimon/src/test/java/org/apache/doris/connector/paimon/PaimonConnectorMetadataTablePropertiesTest.java
@@ -25,6 +25,7 @@ import org.apache.paimon.catalog.Identifier;
import org.apache.paimon.fs.local.LocalFileIO;
import org.apache.paimon.schema.Schema;
import org.apache.paimon.schema.SchemaChange;
+import org.apache.paimon.schema.SchemaManager;
import org.apache.paimon.table.FileStoreTable;
import org.apache.paimon.types.DataTypes;
import org.junit.jupiter.api.Assertions;
@@ -114,6 +115,54 @@ public class PaimonConnectorMetadataTablePropertiesTest {
}
}
+ @Test
+ public void setTablePropertiesRejectsPathBeforeRemoteAlter() {
+ RecordingPaimonCatalogOps ops = new RecordingPaimonCatalogOps();
+ RecordingConnectorContext context = new RecordingConnectorContext();
+ PaimonConnectorMetadata metadata = new PaimonConnectorMetadata(
+ ops, PaimonCatalogProperties.of(Collections.emptyMap()),
context);
+ Map<String, String> properties = new LinkedHashMap<>();
+ properties.put("snapshot.num-retained.max", "10");
+ properties.put("PATH", "s3://warehouse/relocated_table");
+
+ DorisConnectorException e =
Assertions.assertThrows(DorisConnectorException.class,
+ () -> metadata.setTableProperties(null, handle(), properties));
+
+ Assertions.assertTrue(e.getMessage().contains("Change path is not
supported yet"), e.getMessage());
+ Assertions.assertTrue(ops.log.isEmpty(), ops.log.toString());
+ Assertions.assertEquals(0, context.authCount);
+ }
+
+ @Test
+ public void setTablePropertiesPathLeavesTableUnchanged(@TempDir Path
warehouse) throws Exception {
+ try (Catalog catalog = localCatalog(warehouse)) {
+ catalog.createTable(TABLE_ID, Schema.newBuilder()
+ .column("id", DataTypes.INT())
+ .option("snapshot.num-retained.min", "1")
+ .option("snapshot.num-retained.max", "8")
+ .build(), false);
+ long schemaId = schemaId(catalog);
+ Map<String, String> properties = new LinkedHashMap<>();
+ properties.put("snapshot.num-retained.max", "10");
+ properties.put("path",
warehouse.resolve("relocated_table").toUri().toString());
+
+ // Paimon itself would store path in a new schema version without
moving the table; the
+ // catalog then overrides it on load, so check the stored schema
rather than the loaded table.
+ Assertions.assertThrows(DorisConnectorException.class,
+ () -> metadataOver(catalog).setTableProperties(null,
handle(), properties));
+
+ Map<String, String> stored = storedSchemaOptions(catalog);
+ Assertions.assertEquals("8",
stored.get("snapshot.num-retained.max"));
+ Assertions.assertFalse(stored.containsKey("path"),
stored.toString());
+ Assertions.assertEquals(schemaId, schemaId(catalog));
+ }
+ }
+
+ private static Map<String, String> storedSchemaOptions(Catalog catalog)
throws Exception {
+ FileStoreTable table = (FileStoreTable) catalog.getTable(TABLE_ID);
+ return new SchemaManager(LocalFileIO.create(),
table.location()).latest().get().options();
+ }
+
private static Map<String, String> options(String min, String max) {
Map<String, String> properties = new LinkedHashMap<>();
properties.put("snapshot.num-retained.min", min);
diff --git
a/regression-test/data/external_table_p0/paimon/test_paimon_alter_properties.out
b/regression-test/data/external_table_p0/paimon/test_paimon_alter_properties.out
index ba7bb871388..0fbbf7ed1ff 100644
---
a/regression-test/data/external_table_p0/paimon/test_paimon_alter_properties.out
+++
b/regression-test/data/external_table_p0/paimon/test_paimon_alter_properties.out
@@ -28,3 +28,9 @@ snapshot.num-retained.max 8
-- !rejected_options --
snapshot.num-retained.max 8
+-- !path_schema --
+2
+
+-- !path_options --
+snapshot.num-retained.max 8
+
diff --git
a/regression-test/suites/external_table_p0/iceberg/test_iceberg_jdbc_catalog.groovy
b/regression-test/suites/external_table_p0/iceberg/test_iceberg_jdbc_catalog.groovy
index 4d3f44e3cb9..c04e3404938 100644
---
a/regression-test/suites/external_table_p0/iceberg/test_iceberg_jdbc_catalog.groovy
+++
b/regression-test/suites/external_table_p0/iceberg/test_iceberg_jdbc_catalog.groovy
@@ -157,7 +157,15 @@ suite("test_iceberg_jdbc_catalog", "p0,external") {
// Test: Create database
sql """DROP DATABASE IF EXISTS ${db_name} FORCE"""
- sql """CREATE DATABASE ${db_name}"""
+ // A JDBC catalog stores namespace properties, but would only record a
location without placing
+ // new tables under it.
+ test {
+ sql """CREATE DATABASE ${db_name} PROPERTIES (
+ 'location' = 's3://warehouse/rejected_database_location/'
+ )"""
+ exception "database property 'location' for iceberg catalog type:
jdbc"
+ }
+ sql """CREATE DATABASE ${db_name} PROPERTIES ('owner' = 'doris')"""
def databases = sql """SHOW DATABASES"""
assertTrue(databases.toString().contains(db_name))
@@ -315,7 +323,7 @@ suite("test_iceberg_jdbc_catalog", "p0,external") {
String mysql_db_name = "mysql_test_db"
sql """DROP DATABASE IF EXISTS ${mysql_db_name} FORCE"""
- sql """CREATE DATABASE ${mysql_db_name}"""
+ sql """CREATE DATABASE ${mysql_db_name} PROPERTIES ('owner' =
'doris')"""
sql """USE ${mysql_db_name}"""
sql """DROP TABLE IF EXISTS test_mysql_catalog"""
diff --git
a/regression-test/suites/external_table_p0/paimon/test_paimon_alter_properties.groovy
b/regression-test/suites/external_table_p0/paimon/test_paimon_alter_properties.groovy
index d504d89a0f6..187177b1bc1 100644
---
a/regression-test/suites/external_table_p0/paimon/test_paimon_alter_properties.groovy
+++
b/regression-test/suites/external_table_p0/paimon/test_paimon_alter_properties.groovy
@@ -119,4 +119,16 @@ suite("test_paimon_alter_properties",
"p0,external,paimon") {
SELECT `key`, value FROM `paimon_alter_properties\$options`
WHERE `key` IN ('snapshot.num-retained.max',
'fields.missing.sequence-group')
"""
+
+ // Paimon keeps the table location in its path option, but changing it
does not relocate the table.
+ // Doris rejects it before the remote alter, so no schema version is
written and path is not stored.
+ test {
+ sql """ALTER TABLE `paimon_alter_properties` SET ('PATH' =
's3://warehouse/relocated_table')"""
+ exception "Change path is not supported yet"
+ }
+ order_qt_path_schema """SELECT MAX(schema_id) FROM
`paimon_alter_properties\$schemas`"""
+ order_qt_path_options """
+ SELECT `key`, value FROM `paimon_alter_properties\$options`
+ WHERE lower(`key`) IN ('path', 'snapshot.num-retained.max')
+ """
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
