This is an automated email from the ASF dual-hosted git repository.
fanng pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 8de31cdb3 [#5094] Fix(catalog-lakehouse-paimon): make some table
properties from reserved to immutable (#5095)
8de31cdb3 is described below
commit 8de31cdb3cc74a82144c0899a6636a70d6588b66
Author: cai can <94670132+caica...@users.noreply.github.com>
AuthorDate: Fri Oct 11 18:52:50 2024 -0700
[#5094] Fix(catalog-lakehouse-paimon): make some table properties from
reserved to immutable (#5095)
### What changes were proposed in this pull request?
make some table properties from reserved to immutable.
```
merge-engine
sequence.field
rowkind.field
```
### Why are the changes needed?
these table properties are immutable in Paimon.
Fix: https://github.com/apache/gravitino/issues/5094
### Does this PR introduce _any_ user-facing change?
updated related doc.
### How was this patch tested?
modified UT.
Co-authored-by: caican <cai...@xiaomi.com>
---
.../paimon/PaimonTablePropertiesMetadata.java | 10 ++-
.../lakehouse/paimon/TestGravitinoPaimonTable.java | 73 +++++++++++++---------
docs/lakehouse-paimon-catalog.md | 9 ++-
3 files changed, 59 insertions(+), 33 deletions(-)
diff --git
a/catalogs/catalog-lakehouse-paimon/src/main/java/org/apache/gravitino/catalog/lakehouse/paimon/PaimonTablePropertiesMetadata.java
b/catalogs/catalog-lakehouse-paimon/src/main/java/org/apache/gravitino/catalog/lakehouse/paimon/PaimonTablePropertiesMetadata.java
index 1c57e5b2c..671dd9d66 100644
---
a/catalogs/catalog-lakehouse-paimon/src/main/java/org/apache/gravitino/catalog/lakehouse/paimon/PaimonTablePropertiesMetadata.java
+++
b/catalogs/catalog-lakehouse-paimon/src/main/java/org/apache/gravitino/catalog/lakehouse/paimon/PaimonTablePropertiesMetadata.java
@@ -18,6 +18,7 @@
*/
package org.apache.gravitino.catalog.lakehouse.paimon;
+import static
org.apache.gravitino.connector.PropertyEntry.stringImmutablePropertyEntry;
import static
org.apache.gravitino.connector.PropertyEntry.stringReservedPropertyEntry;
import com.google.common.collect.ImmutableList;
@@ -51,9 +52,12 @@ public class PaimonTablePropertiesMetadata extends
BasePropertiesMetadata {
stringReservedPropertyEntry(COMMENT, "The table comment", true),
stringReservedPropertyEntry(OWNER, "The table owner", false),
stringReservedPropertyEntry(BUCKET_KEY, "The table bucket key",
false),
- stringReservedPropertyEntry(MERGE_ENGINE, "The table merge
engine", false),
- stringReservedPropertyEntry(SEQUENCE_FIELD, "The table sequence
field", false),
- stringReservedPropertyEntry(ROWKIND_FIELD, "The table rowkind
field", false),
+ stringImmutablePropertyEntry(
+ MERGE_ENGINE, "The table merge engine", false, null, false,
false),
+ stringImmutablePropertyEntry(
+ SEQUENCE_FIELD, "The table sequence field", false, null,
false, false),
+ stringImmutablePropertyEntry(
+ ROWKIND_FIELD, "The table rowkind field", false, null, false,
false),
stringReservedPropertyEntry(PRIMARY_KEY, "The table primary key",
false),
stringReservedPropertyEntry(PARTITION, "The table partition",
false));
PROPERTIES_METADATA = Maps.uniqueIndex(propertyEntries,
PropertyEntry::getName);
diff --git
a/catalogs/catalog-lakehouse-paimon/src/test/java/org/apache/gravitino/catalog/lakehouse/paimon/TestGravitinoPaimonTable.java
b/catalogs/catalog-lakehouse-paimon/src/test/java/org/apache/gravitino/catalog/lakehouse/paimon/TestGravitinoPaimonTable.java
index 05e36219d..466ecb345 100644
---
a/catalogs/catalog-lakehouse-paimon/src/test/java/org/apache/gravitino/catalog/lakehouse/paimon/TestGravitinoPaimonTable.java
+++
b/catalogs/catalog-lakehouse-paimon/src/test/java/org/apache/gravitino/catalog/lakehouse/paimon/TestGravitinoPaimonTable.java
@@ -382,16 +382,41 @@ public class TestGravitinoPaimonTable {
try (PaimonCatalogOperations ops = new PaimonCatalogOperations()) {
ops.initialize(
initBackendCatalogProperties(), entity.toCatalogInfo(),
PAIMON_PROPERTIES_METADATA);
- Map<String, String> map = Maps.newHashMap();
- map.put(PaimonTablePropertiesMetadata.COMMENT, "test");
- map.put(PaimonTablePropertiesMetadata.OWNER, "test");
- map.put(PaimonTablePropertiesMetadata.BUCKET_KEY, "test");
- map.put(PaimonTablePropertiesMetadata.MERGE_ENGINE, "test");
- map.put(PaimonTablePropertiesMetadata.SEQUENCE_FIELD, "test");
- map.put(PaimonTablePropertiesMetadata.ROWKIND_FIELD, "test");
- map.put(PaimonTablePropertiesMetadata.PRIMARY_KEY, "test");
- map.put(PaimonTablePropertiesMetadata.PARTITION, "test");
- for (Map.Entry<String, String> entry : map.entrySet()) {
+ HashMap<String, String> reservedProps =
+ new HashMap<String, String>() {
+ {
+ put(PaimonTablePropertiesMetadata.COMMENT, "test");
+ put(PaimonTablePropertiesMetadata.OWNER, "test");
+ put(PaimonTablePropertiesMetadata.BUCKET_KEY, "test");
+ put(PaimonTablePropertiesMetadata.PRIMARY_KEY, "test");
+ put(PaimonTablePropertiesMetadata.PARTITION, "test");
+ }
+ };
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ PropertiesMetadataHelpers.validatePropertyForCreate(
+ paimonCatalog.tablePropertiesMetadata(), reservedProps));
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ PropertiesMetadataHelpers.validatePropertyForAlter(
+ paimonCatalog.tablePropertiesMetadata(), reservedProps,
Collections.emptyMap()));
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ PropertiesMetadataHelpers.validatePropertyForAlter(
+ paimonCatalog.tablePropertiesMetadata(),
Collections.emptyMap(), reservedProps));
+
+ Map<String, String> immutableProps =
+ new HashMap<String, String>() {
+ {
+ put(PaimonTablePropertiesMetadata.MERGE_ENGINE, "test");
+ put(PaimonTablePropertiesMetadata.SEQUENCE_FIELD, "test");
+ put(PaimonTablePropertiesMetadata.ROWKIND_FIELD, "test");
+ }
+ };
+ for (Map.Entry<String, String> entry : immutableProps.entrySet()) {
HashMap<String, String> properties =
new HashMap<String, String>() {
{
@@ -399,26 +424,18 @@ public class TestGravitinoPaimonTable {
}
};
PropertiesMetadata metadata = paimonCatalog.tablePropertiesMetadata();
+ Assertions.assertDoesNotThrow(
+ () ->
PropertiesMetadataHelpers.validatePropertyForCreate(metadata, properties));
Assertions.assertThrows(
IllegalArgumentException.class,
- () ->
PropertiesMetadataHelpers.validatePropertyForCreate(metadata, properties));
- }
-
- map = Maps.newHashMap();
- map.put("key1", "val1");
- map.put("key2", "val2");
- for (Map.Entry<String, String> entry : map.entrySet()) {
- HashMap<String, String> properties =
- new HashMap<String, String>() {
- {
- put(entry.getKey(), entry.getValue());
- }
- };
- PropertiesMetadata metadata = paimonCatalog.tablePropertiesMetadata();
- Assertions.assertDoesNotThrow(
- () -> {
- PropertiesMetadataHelpers.validatePropertyForCreate(metadata,
properties);
- });
+ () ->
+ PropertiesMetadataHelpers.validatePropertyForAlter(
+ metadata, properties, Collections.emptyMap()));
+ Assertions.assertThrows(
+ IllegalArgumentException.class,
+ () ->
+ PropertiesMetadataHelpers.validatePropertyForAlter(
+ metadata, Collections.emptyMap(), properties));
}
}
}
diff --git a/docs/lakehouse-paimon-catalog.md b/docs/lakehouse-paimon-catalog.md
index b9c2772bc..2a8226668 100644
--- a/docs/lakehouse-paimon-catalog.md
+++ b/docs/lakehouse-paimon-catalog.md
@@ -175,11 +175,16 @@ The Gravitino server doesn't allow passing the following
reserved fields.
| `comment` | The table comment.
|
| `owner` | The table owner.
|
| `bucket-key` | The table bucket-key.
|
+| `primary-key` | The table primary-key.
|
+| `partition` | The table partition.
|
+
+The Gravitino server doesn't allow the following immutable fields to be
modified, but allows them to be specified when creating a new table.
+
+| Configuration item | Description
|
+|------------------------------------|--------------------------------------------------------------|
| `merge-engine` | The table merge-engine.
|
| `sequence.field` | The table sequence.field.
|
| `rowkind.field` | The table rowkind.field.
|
-| `primary-key` | The table primary-key.
|
-| `partition` | The table partition.
|
### Table operations
