From 3715b00d2b097d08af7538c1f6d16705958fb46a Mon Sep 17 00:00:00 2001 From: mchades Date: Thu, 22 Oct 2026 10:35:30 +0800 Subject: [PATCH 1/3] feat(fileset): Support managed and external type conversion --- .../apache/gravitino/file/FilesetChange.java | 61 ++++++++- .../gravitino/file/TestFilesetChange.java | 18 +++ .../fileset/FilesetCatalogOperations.java | 5 +- .../fileset/TestFilesetCatalogOperations.java | 118 ++++++++++++++++++ .../gravitino/client/DTOConverters.java | 3 + .../gravitino/client/TestFilesetCatalog.java | 25 ++++ .../gravitino/api/file/fileset_change.py | 42 ++++++- .../gravitino/client/fileset_catalog.py | 2 + .../dto/requests/fileset_update_request.py | 20 +++ .../unittests/test_fileset_type_change.py | 71 +++++++++++ .../dto/requests/FilesetUpdateRequest.java | 33 +++++ .../requests/TestFilesetUpdatesRequest.java | 45 +++++++ docs/filesets.md | 16 ++- ...manage-fileset-metadata-using-gravitino.md | 45 +++++++ docs/open-api/filesets.yaml | 28 +++++ .../web/rest/TestFilesetOperations.java | 33 +++++ 16 files changed, 556 insertions(+), 9 deletions(-) create mode 100644 clients/client-python/tests/unittests/test_fileset_type_change.py diff --git a/api/src/main/java/org/apache/gravitino/file/FilesetChange.java b/api/src/main/java/org/apache/gravitino/file/FilesetChange.java index 5d5e5aecb5a..f2b207f78a4 100644 --- a/api/src/main/java/org/apache/gravitino/file/FilesetChange.java +++ b/api/src/main/java/org/apache/gravitino/file/FilesetChange.java @@ -25,7 +25,8 @@ /** * A fileset change is a change to a fileset. It can be used to rename a fileset, update the comment - * of a fileset, set a property and value pair for a fileset, or remove a property from a fileset. + * or type of a fileset, set a property and value pair for a fileset, or remove a property from a + * fileset. */ @Evolving public interface FilesetChange { @@ -50,6 +51,22 @@ static FilesetChange updateComment(String newComment) { return new UpdateFilesetComment(newComment); } + /** + * Creates a new fileset change to update the fileset type. + * + *

Changing the type preserves the fileset's identity, storage locations, and existing files. + * Changing to {@link Fileset.Type#EXTERNAL} relinquishes management of the data, so dropping the + * fileset retains it. Changing to {@link Fileset.Type#MANAGED} accepts management of the data, + * including deleting it when the fileset is dropped. Setting the current type is allowed. + * + * @param newType The new type of the fileset. + * @return The fileset change. + * @throws IllegalArgumentException if the new type is null. + */ + static FilesetChange updateType(Fileset.Type newType) { + return new UpdateFilesetType(newType); + } + /** * Creates a new fileset change to set the property and value for the fileset. * @@ -215,6 +232,48 @@ public String toString() { } } + /** A fileset change to update the fileset type without changing its data or locations. */ + final class UpdateFilesetType implements FilesetChange { + private final Fileset.Type newType; + + private UpdateFilesetType(Fileset.Type newType) { + if (newType == null) { + throw new IllegalArgumentException("New fileset type cannot be null"); + } + this.newType = newType; + } + + /** + * Returns the new type of the fileset. + * + * @return The new fileset type. + */ + public Fileset.Type getNewType() { + return newType; + } + + /** {@inheritDoc} */ + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + UpdateFilesetType that = (UpdateFilesetType) o; + return newType == that.newType; + } + + /** {@inheritDoc} */ + @Override + public int hashCode() { + return Objects.hash(newType); + } + + /** {@inheritDoc} */ + @Override + public String toString() { + return "UPDATEFILESETTYPE " + newType; + } + } + /** A fileset change to set the property and value for the fileset. */ final class SetProperty implements FilesetChange { private final String property; diff --git a/api/src/test/java/org/apache/gravitino/file/TestFilesetChange.java b/api/src/test/java/org/apache/gravitino/file/TestFilesetChange.java index fd8a96ffe1e..df17550e6ef 100644 --- a/api/src/test/java/org/apache/gravitino/file/TestFilesetChange.java +++ b/api/src/test/java/org/apache/gravitino/file/TestFilesetChange.java @@ -23,6 +23,24 @@ import org.junit.jupiter.api.Test; public class TestFilesetChange { + @Test + void testUpdateTypeChange() { + for (Fileset.Type type : Fileset.Type.values()) { + FilesetChange.UpdateFilesetType change = + (FilesetChange.UpdateFilesetType) FilesetChange.updateType(type); + Assertions.assertEquals(type, change.getNewType()); + Assertions.assertEquals(change, FilesetChange.updateType(type)); + Assertions.assertEquals(change.hashCode(), FilesetChange.updateType(type).hashCode()); + Assertions.assertNotEquals(change, null); + Assertions.assertNotEquals(change, FilesetChange.rename("fileset")); + Assertions.assertEquals("UPDATEFILESETTYPE " + type, change.toString()); + } + Assertions.assertNotEquals( + FilesetChange.updateType(Fileset.Type.MANAGED), + FilesetChange.updateType(Fileset.Type.EXTERNAL)); + Assertions.assertThrows(IllegalArgumentException.class, () -> FilesetChange.updateType(null)); + } + @Test void testRemovePropertyChange() { String property = "property1"; diff --git a/catalogs/catalog-fileset/src/main/java/org/apache/gravitino/catalog/fileset/FilesetCatalogOperations.java b/catalogs/catalog-fileset/src/main/java/org/apache/gravitino/catalog/fileset/FilesetCatalogOperations.java index f1de5114b1c..951c27a0881 100644 --- a/catalogs/catalog-fileset/src/main/java/org/apache/gravitino/catalog/fileset/FilesetCatalogOperations.java +++ b/catalogs/catalog-fileset/src/main/java/org/apache/gravitino/catalog/fileset/FilesetCatalogOperations.java @@ -1287,6 +1287,7 @@ private FilesetEntity updateFilesetEntity( : Maps.newHashMap(filesetEntity.properties()); String newName = ident.name(); String newComment = filesetEntity.comment(); + Fileset.Type newType = filesetEntity.filesetType(); for (FilesetChange change : changes) { if (change instanceof FilesetChange.SetProperty) { @@ -1299,6 +1300,8 @@ private FilesetEntity updateFilesetEntity( newName = ((FilesetChange.RenameFileset) change).getNewName(); } else if (change instanceof FilesetChange.UpdateFilesetComment) { newComment = ((FilesetChange.UpdateFilesetComment) change).getNewComment(); + } else if (change instanceof FilesetChange.UpdateFilesetType) { + newType = ((FilesetChange.UpdateFilesetType) change).getNewType(); } else if (change instanceof FilesetChange.RemoveComment) { newComment = null; } else { @@ -1312,7 +1315,7 @@ private FilesetEntity updateFilesetEntity( .withNamespace(ident.namespace()) .withId(filesetEntity.id()) .withComment(newComment) - .withFilesetType(filesetEntity.filesetType()) + .withFilesetType(newType) .withStorageLocations(filesetEntity.storageLocations()) .withProperties(props) .withAuditInfo( diff --git a/catalogs/catalog-fileset/src/test/java/org/apache/gravitino/catalog/fileset/TestFilesetCatalogOperations.java b/catalogs/catalog-fileset/src/test/java/org/apache/gravitino/catalog/fileset/TestFilesetCatalogOperations.java index f0d50309656..110c87e2769 100644 --- a/catalogs/catalog-fileset/src/test/java/org/apache/gravitino/catalog/fileset/TestFilesetCatalogOperations.java +++ b/catalogs/catalog-fileset/src/test/java/org/apache/gravitino/catalog/fileset/TestFilesetCatalogOperations.java @@ -162,6 +162,7 @@ import org.junit.jupiter.api.io.TempDir; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.CsvSource; import org.junit.jupiter.params.provider.EnumSource; import org.junit.jupiter.params.provider.MethodSource; import org.junit.jupiter.params.provider.ValueSource; @@ -1359,6 +1360,123 @@ public void testAlterFilesetProperties() throws IOException { } } + @ParameterizedTest + @CsvSource({"EXTERNAL,false", "MANAGED,false", "EXTERNAL,true", "MANAGED,true"}) + void testUpdateFilesetTypePreservesDataAndControlsDrop( + Fileset.Type targetType, boolean cascade, @TempDir File root) throws IOException { + String schemaName = "schema" + generateTestId(); + createSchema(schemaName, "schema comment", null, null); + NameIdentifier ident = NameIdentifier.of("m1", "c1", schemaName, "fileset"); + Fileset.Type originalType = + targetType == Fileset.Type.MANAGED ? Fileset.Type.EXTERNAL : Fileset.Type.MANAGED; + File primary = new File(root, "primary"); + File backup = new File(root, "backup"); + Files.createDirectories(primary.toPath()); + Files.createDirectories(backup.toPath()); + Map locations = + ImmutableMap.of("primary", primary.toURI().toString(), "backup", backup.toURI().toString()); + Map props = + StringIdentifier.newPropertiesWithId( + StringIdentifier.fromId(idGenerator.nextId()), + ImmutableMap.of(PROPERTY_DEFAULT_LOCATION_NAME, "primary", "k1", "v1")); + + try (FilesetCatalogOperations ops = new FilesetCatalogOperations(store, secretManager)) { + ops.initialize(Maps.newHashMap(), randomCatalogInfo(), FILESET_PROPERTIES_METADATA); + ops.createMultipleLocationFileset(ident, "comment", originalType, locations, props); + for (File directory : Arrays.asList(primary, backup)) { + Files.writeString(new File(directory, "data.txt").toPath(), "existing data"); + } + FilesetEntity before = store.get(ident, Entity.EntityType.FILESET, FilesetEntity.class); + + // Repeating the conversion must preserve both metadata and existing data. + for (int attempt = 0; attempt < 2; attempt++) { + Fileset updated = ops.alterFileset(ident, FilesetChange.updateType(targetType)); + Assertions.assertEquals(targetType, updated.type()); + Assertions.assertEquals(before.name(), updated.name()); + Assertions.assertEquals(before.comment(), updated.comment()); + Assertions.assertEquals(before.storageLocations(), updated.storageLocations()); + Assertions.assertEquals(before.properties(), updated.properties()); + FilesetEntity persisted = store.get(ident, Entity.EntityType.FILESET, FilesetEntity.class); + Assertions.assertEquals(before.id(), persisted.id()); + Assertions.assertEquals(before.namespace(), persisted.namespace()); + Assertions.assertEquals(targetType, persisted.filesetType()); + Assertions.assertEquals(before.auditInfo().creator(), persisted.auditInfo().creator()); + Assertions.assertEquals( + before.auditInfo().createTime(), persisted.auditInfo().createTime()); + Assertions.assertNotNull(persisted.auditInfo().lastModifiedTime()); + for (File directory : Arrays.asList(primary, backup)) { + Assertions.assertEquals( + "existing data", Files.readString(new File(directory, "data.txt").toPath())); + } + } + + if (cascade) { + Assertions.assertTrue(ops.dropSchema(NameIdentifier.of("m1", "c1", schemaName), true)); + } else { + Assertions.assertTrue(ops.dropFileset(ident)); + } + Assertions.assertFalse(store.exists(ident, Entity.EntityType.FILESET)); + for (File directory : Arrays.asList(primary, backup)) { + if (targetType == Fileset.Type.EXTERNAL) { + Assertions.assertEquals( + "existing data", Files.readString(new File(directory, "data.txt").toPath())); + } else { + Assertions.assertFalse(directory.exists()); + } + } + } + } + + @ParameterizedTest + @ValueSource(booleans = {true, false}) + void testUpdateFilesetTypeWithOtherChanges(boolean disableFsOps) throws IOException { + String schemaName = "schema" + generateTestId(); + createSchema(schemaName, "comment", null, null, true); + NameIdentifier ident = NameIdentifier.of("m1", "c1", schemaName, "fileset"); + File missingLocation = new File(UNFORMALIZED_TEST_ROOT_PATH, "missing-" + schemaName); + createFileset( + "fileset", + schemaName, + "comment", + Fileset.Type.EXTERNAL, + null, + missingLocation.toURI().toString(), + true); + FilesetEntity before = store.get(ident, Entity.EntityType.FILESET, FilesetEntity.class); + try (FilesetCatalogOperations ops = new FilesetCatalogOperations(store, secretManager)) { + ops.initialize( + ImmutableMap.of(DISABLE_FILESYSTEM_OPS, Boolean.toString(disableFsOps)), + randomCatalogInfo(), + FILESET_PROPERTIES_METADATA); + Fileset updated = + ops.alterFileset( + ident, + FilesetChange.updateType(Fileset.Type.MANAGED), + FilesetChange.rename("renamed"), + FilesetChange.updateComment("updated"), + FilesetChange.setProperty("k1", "v1")); + NameIdentifier renamed = NameIdentifier.of(ident.namespace(), "renamed"); + Assertions.assertEquals(Fileset.Type.MANAGED, updated.type()); + Assertions.assertEquals("renamed", updated.name()); + Assertions.assertEquals("updated", updated.comment()); + Assertions.assertEquals("v1", updated.properties().get("k1")); + Assertions.assertFalse(store.exists(ident, Entity.EntityType.FILESET)); + Assertions.assertEquals( + before.id(), store.get(renamed, Entity.EntityType.FILESET, FilesetEntity.class).id()); + Assertions.assertEquals(before.storageLocations(), updated.storageLocations()); + Assertions.assertFalse(missingLocation.exists()); + + // Multiple type changes follow request order, like other fileset changes. + Fileset reverted = + ops.alterFileset( + renamed, + FilesetChange.updateType(Fileset.Type.MANAGED), + FilesetChange.updateType(Fileset.Type.EXTERNAL)); + Assertions.assertEquals(Fileset.Type.EXTERNAL, reverted.type()); + Assertions.assertTrue(ops.dropFileset(renamed)); + } + } + @Test public void testFormalizePath() throws IOException, IllegalAccessException, InterruptedException { diff --git a/clients/client-java/src/main/java/org/apache/gravitino/client/DTOConverters.java b/clients/client-java/src/main/java/org/apache/gravitino/client/DTOConverters.java index 9bc6fdd0ff8..f4ec27b1049 100644 --- a/clients/client-java/src/main/java/org/apache/gravitino/client/DTOConverters.java +++ b/clients/client-java/src/main/java/org/apache/gravitino/client/DTOConverters.java @@ -286,6 +286,9 @@ static FilesetUpdateRequest toFilesetUpdateRequest(FilesetChange change) { } else if (change instanceof FilesetChange.UpdateFilesetComment) { return new FilesetUpdateRequest.UpdateFilesetCommentRequest( ((FilesetChange.UpdateFilesetComment) change).getNewComment()); + } else if (change instanceof FilesetChange.UpdateFilesetType) { + return new FilesetUpdateRequest.UpdateFilesetTypeRequest( + ((FilesetChange.UpdateFilesetType) change).getNewType()); } else if (change instanceof FilesetChange.RemoveComment) { return new FilesetUpdateRequest.RemoveFilesetCommentRequest(); } else if (change instanceof FilesetChange.SetProperty) { diff --git a/clients/client-java/src/test/java/org/apache/gravitino/client/TestFilesetCatalog.java b/clients/client-java/src/test/java/org/apache/gravitino/client/TestFilesetCatalog.java index f71bdce0e46..7bf900c16cc 100644 --- a/clients/client-java/src/test/java/org/apache/gravitino/client/TestFilesetCatalog.java +++ b/clients/client-java/src/test/java/org/apache/gravitino/client/TestFilesetCatalog.java @@ -62,6 +62,7 @@ import org.apache.gravitino.exceptions.NoSuchSchemaException; import org.apache.gravitino.exceptions.NotFoundException; import org.apache.gravitino.file.Fileset; +import org.apache.gravitino.file.FilesetChange; import org.apache.hc.core5.http.Method; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; @@ -429,6 +430,30 @@ public void testAlterFileset() throws JsonProcessingException { "internal error"); } + @Test + public void testAlterFilesetType() throws JsonProcessingException { + NameIdentifier ident = NameIdentifier.of("schema1", "fileset1"); + String path = + withSlash( + FilesetCatalog.formatFilesetRequestPath( + Namespace.of(metalakeName, catalogName, "schema1")) + + "/fileset1"); + for (Fileset.Type type : Fileset.Type.values()) { + FilesetDTO expected = + mockFilesetDTO("fileset1", type, "comment", "mock location", ImmutableMap.of("k1", "v1")); + FilesetUpdateRequest request = new FilesetUpdateRequest.UpdateFilesetTypeRequest(type); + buildMockResource( + Method.PUT, + path, + new FilesetUpdatesRequest(ImmutableList.of(request)), + new FilesetResponse(expected), + SC_OK); + Fileset actual = + catalog.asFilesetCatalog().alterFileset(ident, FilesetChange.updateType(type)); + assertFileset(expected, actual); + } + } + @Test public void testGetFileLocation() throws JsonProcessingException { NameIdentifier fileset = NameIdentifier.of(metalakeName, catalogName, "schema1", "fileset1"); diff --git a/clients/client-python/gravitino/api/file/fileset_change.py b/clients/client-python/gravitino/api/file/fileset_change.py index 7bc2f7bc6df..ba57e6a90d5 100644 --- a/clients/client-python/gravitino/api/file/fileset_change.py +++ b/clients/client-python/gravitino/api/file/fileset_change.py @@ -20,12 +20,13 @@ from dataclasses_json import config +from gravitino.api.file.fileset import Fileset from gravitino.api.secret import SecretBinding, SecretReference class FilesetChange(ABC): """A fileset change is a change to a fileset. It can be used to rename a fileset, update the comment - of a fileset, set a property and value pair for a fileset, or remove a property from a fileset. + or type of a fileset, set a property and value pair for a fileset, or remove a property from a fileset. """ @staticmethod @@ -52,6 +53,25 @@ def update_comment(new_comment): """ return FilesetChange.UpdateFilesetComment(new_comment) + @staticmethod + def update_type(new_type: Fileset.Type): + """Changes the fileset type without changing its identity, locations, or files. + + EXTERNAL retains data when the fileset is dropped. MANAGED accepts management + of the data, including deleting it when the fileset is dropped. Setting the + current type is allowed. + + Args: + new_type: The new fileset type. + + Returns: + The fileset change. + + Raises: + ValueError: If new_type is not a Fileset.Type. + """ + return FilesetChange.UpdateFilesetType(new_type) + @staticmethod def set_property(fileset_property, value): """Creates a new fileset change to set the property and value for the fileset. @@ -191,6 +211,26 @@ def __str__(self): """ return f"UPDATEFILESETCOMMENT {self._new_comment}" + @dataclass + class UpdateFilesetType: + """A fileset change to update the type without changing data or locations.""" + + _new_type: Fileset.Type = field(metadata=config(field_name="newType")) + + def __post_init__(self): + if not isinstance(self._new_type, Fileset.Type): + raise ValueError("New fileset type must be a Fileset.Type") + + def new_type(self) -> Fileset.Type: + """Returns the new fileset type.""" + return self._new_type + + def __hash__(self): + return hash(self._new_type) + + def __str__(self): + return f"UPDATEFILESETTYPE {self._new_type.name}" + @dataclass class SetProperty: """A fileset change to set the property and value for the fileset.""" diff --git a/clients/client-python/gravitino/client/fileset_catalog.py b/clients/client-python/gravitino/client/fileset_catalog.py index 4d8fed43b93..ef9e0cc8aa9 100644 --- a/clients/client-python/gravitino/client/fileset_catalog.py +++ b/clients/client-python/gravitino/client/fileset_catalog.py @@ -388,6 +388,8 @@ def to_fileset_update_request(change: FilesetChange): return FilesetUpdateRequest.UpdateFilesetCommentRequest( change.new_comment() ) + if isinstance(change, FilesetChange.UpdateFilesetType): + return FilesetUpdateRequest.UpdateFilesetTypeRequest(change.new_type()) if isinstance(change, FilesetChange.SetProperty): return FilesetUpdateRequest.SetFilesetPropertyRequest( change.property(), change.value() diff --git a/clients/client-python/gravitino/dto/requests/fileset_update_request.py b/clients/client-python/gravitino/dto/requests/fileset_update_request.py index 182ab5d46c8..204bab6e21d 100644 --- a/clients/client-python/gravitino/dto/requests/fileset_update_request.py +++ b/clients/client-python/gravitino/dto/requests/fileset_update_request.py @@ -21,6 +21,7 @@ from dataclasses_json import config +from gravitino.api.file.fileset import Fileset from gravitino.api.file.fileset_change import FilesetChange from gravitino.api.secret import SecretBinding, SecretReference from gravitino.rest.rest_message import RESTRequest @@ -88,6 +89,25 @@ def fileset_change(self): """Returns the fileset change""" return FilesetChange.update_comment(self._new_comment) + @dataclass + class UpdateFilesetTypeRequest(FilesetUpdateRequestBase): + """Represents a request to change the type of a fileset.""" + + _new_type: Fileset.Type = field(metadata=config(field_name="newType")) + + def __init__(self, new_type: Fileset.Type): + super().__init__("updateType") + self._new_type = new_type + + def validate(self): + """Rejects a missing or invalid target type.""" + if not isinstance(self._new_type, Fileset.Type): + raise ValueError('"newType" field must be a Fileset.Type') + + def fileset_change(self): + """Returns the fileset type change.""" + return FilesetChange.update_type(self._new_type) + @dataclass class SetFilesetPropertyRequest(FilesetUpdateRequestBase): """Represents a request to set a property on a Fileset.""" diff --git a/clients/client-python/tests/unittests/test_fileset_type_change.py b/clients/client-python/tests/unittests/test_fileset_type_change.py new file mode 100644 index 00000000000..17e83a5c572 --- /dev/null +++ b/clients/client-python/tests/unittests/test_fileset_type_change.py @@ -0,0 +1,71 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +import json +import unittest + +from gravitino.api.file.fileset import Fileset +from gravitino.api.file.fileset_change import FilesetChange +from gravitino.client.fileset_catalog import FilesetCatalog +from gravitino.dto.requests.fileset_update_request import FilesetUpdateRequest +from gravitino.dto.requests.fileset_updates_request import FilesetUpdatesRequest + + +class TestFilesetTypeChange(unittest.TestCase): + def test_update_type_request(self): + for fileset_type in Fileset.Type: + with self.subTest(fileset_type=fileset_type): + change = FilesetChange.update_type(fileset_type) + self.assertEqual(fileset_type, change.new_type()) + self.assertEqual(change, FilesetChange.update_type(fileset_type)) + self.assertEqual( + hash(change), hash(FilesetChange.update_type(fileset_type)) + ) + self.assertEqual(f"UPDATEFILESETTYPE {fileset_type.name}", str(change)) + request = FilesetCatalog.to_fileset_update_request(change) + self.assertEqual(change, request.fileset_change()) + updates = FilesetUpdatesRequest([request]) + updates.validate() + self.assertEqual( + { + "updates": [ + {"@type": "updateType", "newType": fileset_type.value} + ] + }, + json.loads(updates.to_json()), + ) + self.assertNotEqual( + FilesetChange.update_type(Fileset.Type.MANAGED), + FilesetChange.update_type(Fileset.Type.EXTERNAL), + ) + self.assertNotEqual(FilesetChange.update_type(Fileset.Type.MANAGED), None) + self.assertNotEqual( + FilesetChange.update_type(Fileset.Type.MANAGED), + FilesetChange.rename("fileset"), + ) + + def test_reject_invalid_type(self): + for invalid in (None, "managed", "invalid", 1): + with self.subTest(invalid=invalid): + with self.assertRaises(ValueError): + FilesetChange.update_type(invalid) + with self.assertRaises(ValueError): + FilesetChange.UpdateFilesetType(invalid) + request = FilesetUpdateRequest.UpdateFilesetTypeRequest(invalid) + with self.assertRaises(ValueError): + FilesetUpdatesRequest([request]).validate() + with self.assertRaises(ValueError): + request.fileset_change() diff --git a/common/src/main/java/org/apache/gravitino/dto/requests/FilesetUpdateRequest.java b/common/src/main/java/org/apache/gravitino/dto/requests/FilesetUpdateRequest.java index e811c5ca5b2..fe0dd4edd43 100644 --- a/common/src/main/java/org/apache/gravitino/dto/requests/FilesetUpdateRequest.java +++ b/common/src/main/java/org/apache/gravitino/dto/requests/FilesetUpdateRequest.java @@ -31,6 +31,7 @@ import lombok.NoArgsConstructor; import lombok.ToString; import org.apache.commons.lang3.StringUtils; +import org.apache.gravitino.file.Fileset; import org.apache.gravitino.file.FilesetChange; import org.apache.gravitino.rest.RESTRequest; import org.apache.gravitino.secret.SecretBinding; @@ -44,6 +45,9 @@ @JsonSubTypes.Type( value = FilesetUpdateRequest.UpdateFilesetCommentRequest.class, name = "updateComment"), + @JsonSubTypes.Type( + value = FilesetUpdateRequest.UpdateFilesetTypeRequest.class, + name = "updateType"), @JsonSubTypes.Type( value = FilesetUpdateRequest.RemoveFilesetCommentRequest.class, name = "removeComment"), @@ -126,6 +130,35 @@ public FilesetChange filesetChange() { public void validate() throws IllegalArgumentException {} } + /** The fileset update request for changing the type of a fileset. */ + @EqualsAndHashCode + @NoArgsConstructor(force = true) + @AllArgsConstructor + @ToString + class UpdateFilesetTypeRequest implements FilesetUpdateRequest { + + @Getter + @JsonProperty("newType") + private final Fileset.Type newType; + + /** {@inheritDoc} */ + @Override + public FilesetChange filesetChange() { + return FilesetChange.updateType(newType); + } + + /** + * Validates the request. + * + * @throws IllegalArgumentException if the new type is null. + */ + @Override + public void validate() throws IllegalArgumentException { + Preconditions.checkArgument( + newType != null, "\"newType\" field is required and cannot be null"); + } + } + /** The fileset update request for setting the properties of a fileset. */ @EqualsAndHashCode @NoArgsConstructor(force = true) diff --git a/common/src/test/java/org/apache/gravitino/dto/requests/TestFilesetUpdatesRequest.java b/common/src/test/java/org/apache/gravitino/dto/requests/TestFilesetUpdatesRequest.java index 92283aceca7..2280b8087b2 100644 --- a/common/src/test/java/org/apache/gravitino/dto/requests/TestFilesetUpdatesRequest.java +++ b/common/src/test/java/org/apache/gravitino/dto/requests/TestFilesetUpdatesRequest.java @@ -18,12 +18,57 @@ */ package org.apache.gravitino.dto.requests; +import com.fasterxml.jackson.core.JsonProcessingException; import java.util.Collections; +import java.util.Locale; +import org.apache.gravitino.file.Fileset; +import org.apache.gravitino.file.FilesetChange; +import org.apache.gravitino.json.JsonUtils; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; public class TestFilesetUpdatesRequest { + @Test + void testUpdateTypeSerialization() throws JsonProcessingException { + for (Fileset.Type type : Fileset.Type.values()) { + FilesetUpdateRequest update = new FilesetUpdateRequest.UpdateFilesetTypeRequest(type); + FilesetUpdatesRequest request = new FilesetUpdatesRequest(Collections.singletonList(update)); + request.validate(); + String json = JsonUtils.objectMapper().writeValueAsString(request); + Assertions.assertEquals( + "updateType", JsonUtils.objectMapper().readTree(json).at("/updates/0/@type").asText()); + Assertions.assertEquals( + type.name().toLowerCase(Locale.ROOT), + JsonUtils.objectMapper().readTree(json).at("/updates/0/newType").asText()); + FilesetUpdatesRequest decoded = + JsonUtils.objectMapper().readValue(json, FilesetUpdatesRequest.class); + decoded.validate(); + Assertions.assertEquals(1, decoded.getUpdates().size()); + Assertions.assertEquals( + FilesetChange.updateType(type), decoded.getUpdates().get(0).filesetChange()); + } + } + + @Test + void testUpdateTypeValidation() throws JsonProcessingException { + for (String update : + new String[] { + "{\"@type\":\"updateType\"}", "{\"@type\":\"updateType\",\"newType\":null}" + }) { + FilesetUpdateRequest request = + JsonUtils.objectMapper().readValue(update, FilesetUpdateRequest.class); + Assertions.assertThrows(IllegalArgumentException.class, request::validate); + } + Assertions.assertThrows( + JsonProcessingException.class, + () -> + JsonUtils.objectMapper() + .readValue( + "{\"@type\":\"updateType\",\"newType\":\"unknown\"}", + FilesetUpdateRequest.class)); + } + @Test public void testValidateWithNullUpdates() { // Test that creating a request with null updates throws an exception during validation diff --git a/docs/filesets.md b/docs/filesets.md index 8dd046f5dce..ae160222b90 100644 --- a/docs/filesets.md +++ b/docs/filesets.md @@ -44,8 +44,9 @@ than federating an external one, so no provider is needed when creating it. [Catalogs and Schemas](./catalogs-and-schemas.md). A fileset catalog usually carries a base `location`, and schemas can narrow it further. -**2. Create the fileset.** Give it a name, a type, and a storage location. Creating a managed -fileset creates the directory; pointing at an existing path makes it external. +**2. Create the fileset.** Give it a name, a type, and a storage location. Choose `MANAGED` to let +Gravitino manage the data lifecycle, or `EXTERNAL` to retain the data when the fileset is dropped. +Pointing at an existing path does not automatically make a fileset external. **3. Read it by name.** Engines reach the files through GVFS or the client rather than the raw path. See [How to use GVFS](./how-to-use-gvfs.md). @@ -54,14 +55,17 @@ See [How to use GVFS](./how-to-use-gvfs.md). ### Managed and External -A managed fileset belongs to Gravitino. Creating it creates the directory, and deleting it deletes -the data. +A managed fileset belongs to Gravitino. Creating it creates the directory if needed, and deleting +it deletes the data. An external fileset points at a location that already exists and stays under someone else's control. Deleting it removes the Gravitino record and leaves the files alone. -The distinction only matters at deletion time, and it is the single most important thing to get -right when creating one. +An existing fileset can be converted between the two types using +[`FilesetChange.updateType`](./manage-fileset-metadata-using-gravitino.md#change-the-fileset-type). +Conversion changes metadata only and preserves its identity, locations, and files. Converting to +external relinquishes management of the data; converting to managed accepts deletion of the data +when the fileset is dropped. ### Storage Locations diff --git a/docs/manage-fileset-metadata-using-gravitino.md b/docs/manage-fileset-metadata-using-gravitino.md index 4b725c74ba4..d66a1764943 100755 --- a/docs/manage-fileset-metadata-using-gravitino.md +++ b/docs/manage-fileset-metadata-using-gravitino.md @@ -156,6 +156,7 @@ Changes are applied as a list in one request. Storage locations cannot be change |--------------------|--------------------------------------------------------------|-----------------------------------------------| | Rename | `{"@type":"rename","newName":"fileset_renamed"}` | `FilesetChange.rename("fileset_renamed")` | | Update the comment | `{"@type":"updateComment","newComment":"new_comment"}` | `FilesetChange.updateComment("new_comment")` | +| Update the type | `{"@type":"updateType","newType":"external"}` | `FilesetChange.updateType(Fileset.Type.EXTERNAL)` | | Set a property | `{"@type":"setProperty","property":"key1","value":"value1"}` | `FilesetChange.setProperty("key1", "value1")` | | Remove a property | `{"@type":"removeProperty","property":"key1"}` | `FilesetChange.removeProperty("key1")` | @@ -183,6 +184,50 @@ Fileset fileset = filesets.alterFileset( +#### Change the Fileset Type + +An existing fileset can be changed between `MANAGED` and `EXTERNAL`. This changes metadata only: +its identifier, name, storage locations, properties, and existing files are preserved. Setting the +current type is allowed. + +Change a `MANAGED` fileset to `EXTERNAL` before dropping it to retain its files. Changing an +`EXTERNAL` fileset to `MANAGED` explicitly accepts management of the existing data, including +deleting that data when the fileset is dropped. The same type semantics apply when dropping a +schema with cascade. Conversion does not move, create, or delete files, and does not protect data +from deletion through another managed fileset with an overlapping location. + + + + +```shell +curl -X PUT -H "Accept: application/vnd.gravitino.v1+json" \ + -H "Content-Type: application/json" -d '{ + "updates": [{"@type": "updateType", "newType": "external"}] +}' http://localhost:8090/api/metalakes/example/catalogs/landing/schemas/raw/filesets/raw_events +``` + + + + +```java +Fileset fileset = filesets.alterFileset( + NameIdentifier.of("raw", "raw_events"), + FilesetChange.updateType(Fileset.Type.EXTERNAL)); +``` + + + + +```python +fileset = filesets.alter_fileset( + NameIdentifier.of("raw", "raw_events"), + FilesetChange.update_type(Fileset.Type.EXTERNAL), +) +``` + + + + ### Drop a Fileset Dropping a `MANAGED` fileset deletes its files. Dropping an `EXTERNAL` one removes only the diff --git a/docs/open-api/filesets.yaml b/docs/open-api/filesets.yaml index 39f89cdd57f..673e1047f05 100644 --- a/docs/open-api/filesets.yaml +++ b/docs/open-api/filesets.yaml @@ -393,6 +393,7 @@ components: - $ref: "#/components/schemas/RenameFilesetRequest" - $ref: "#/components/schemas/SetFilesetPropertyRequest" - $ref: "#/components/schemas/UpdateFilesetCommentRequest" + - $ref: "#/components/schemas/UpdateFilesetTypeRequest" - $ref: "#/components/schemas/RemoveFilesetPropertyRequest" - $ref: "#/components/schemas/SetFilesetSecretBindingRequest" - $ref: "#/components/schemas/SetFilesetSecretReferenceRequest" @@ -402,6 +403,7 @@ components: rename: "#/components/schemas/RenameFilesetRequest" setProperty: "#/components/schemas/SetFilesetPropertyRequest" updateComment: "#/components/schemas/UpdateFilesetCommentRequest" + updateType: "#/components/schemas/UpdateFilesetTypeRequest" removeProperty: "#/components/schemas/RemoveFilesetPropertyRequest" setSecretBinding: "#/components/schemas/SetFilesetSecretBindingRequest" setSecretReference: "#/components/schemas/SetFilesetSecretReferenceRequest" @@ -468,6 +470,32 @@ components: "newComment": "new comment" } + UpdateFilesetTypeRequest: + type: object + description: >- + Changes the fileset type without changing its identity, storage locations, or files. + Changing to external retains data on drop. Changing to managed accepts management + of the data, including deleting it on drop. Setting the current type is allowed. + required: + - "@type" + - newType + properties: + "@type": + type: string + description: The type of the update + enum: + - updateType + newType: + type: string + description: The new type of the fileset + enum: + - managed + - external + example: { + "@type": "updateType", + "newType": "external" + } + RemoveFilesetPropertyRequest: type: object required: diff --git a/server/src/test/java/org/apache/gravitino/server/web/rest/TestFilesetOperations.java b/server/src/test/java/org/apache/gravitino/server/web/rest/TestFilesetOperations.java index 6a7b10af981..a5c16b50a17 100644 --- a/server/src/test/java/org/apache/gravitino/server/web/rest/TestFilesetOperations.java +++ b/server/src/test/java/org/apache/gravitino/server/web/rest/TestFilesetOperations.java @@ -82,7 +82,9 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.EnumSource; import org.junit.jupiter.params.provider.MethodSource; +import org.junit.jupiter.params.provider.ValueSource; import org.mockito.Mockito; import org.mockito.stubbing.Answer; @@ -496,6 +498,37 @@ public void testUpdateFilesetComment() { assertUpdateFileset(new FilesetUpdatesRequest(ImmutableList.of(req)), fileset); } + @ParameterizedTest + @EnumSource(Fileset.Type.class) + void testUpdateFilesetType(Fileset.Type type) { + FilesetUpdateRequest req = new FilesetUpdateRequest.UpdateFilesetTypeRequest(type); + Fileset fileset = + mockFileset("fileset1", type, "comment", "mock location", ImmutableMap.of("k1", "v1")); + assertUpdateFileset(new FilesetUpdatesRequest(ImmutableList.of(req)), fileset); + Mockito.verify(dispatcher) + .alterFileset( + NameIdentifier.of(metalake, catalog, schema, "fileset1"), + FilesetChange.updateType(type)); + } + + @ParameterizedTest + @ValueSource( + strings = { + "{\"updates\":[{\"@type\":\"updateType\"}]}", + "{\"updates\":[{\"@type\":\"updateType\",\"newType\":null}]}", + "{\"updates\":[{\"@type\":\"updateType\",\"newType\":\"unknown\"}]}" + }) + void testUpdateFilesetTypeRejectsInvalidRequest(String json) { + try (Response response = + target(filesetPath(metalake, catalog, schema) + "fileset1") + .request(MediaType.APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .put(Entity.entity(json, MediaType.APPLICATION_JSON_TYPE))) { + Assertions.assertEquals(400, response.getStatus()); + Mockito.verifyNoInteractions(dispatcher); + } + } + @Test public void testRemoveFilesetComment() { FilesetUpdateRequest req = new FilesetUpdateRequest.UpdateFilesetCommentRequest(null); From 04f758a1f6c9c8eeace1f2d85a800031918bf254 Mon Sep 17 00:00:00 2001 From: mchades Date: Thu, 22 Oct 2026 11:47:49 +0800 Subject: [PATCH 2/3] docs(fileset): Clarify supported fileset change operations --- .../main/java/org/apache/gravitino/file/FilesetChange.java | 4 ++-- clients/client-python/gravitino/api/file/fileset_change.py | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/api/src/main/java/org/apache/gravitino/file/FilesetChange.java b/api/src/main/java/org/apache/gravitino/file/FilesetChange.java index f2b207f78a4..44c13e93698 100644 --- a/api/src/main/java/org/apache/gravitino/file/FilesetChange.java +++ b/api/src/main/java/org/apache/gravitino/file/FilesetChange.java @@ -25,8 +25,8 @@ /** * A fileset change is a change to a fileset. It can be used to rename a fileset, update the comment - * or type of a fileset, set a property and value pair for a fileset, or remove a property from a - * fileset. + * of a fileset, update the type of a fileset, set a property and value pair for a fileset, or + * remove a property from a fileset. */ @Evolving public interface FilesetChange { diff --git a/clients/client-python/gravitino/api/file/fileset_change.py b/clients/client-python/gravitino/api/file/fileset_change.py index ba57e6a90d5..1bb65859f1e 100644 --- a/clients/client-python/gravitino/api/file/fileset_change.py +++ b/clients/client-python/gravitino/api/file/fileset_change.py @@ -25,8 +25,9 @@ class FilesetChange(ABC): - """A fileset change is a change to a fileset. It can be used to rename a fileset, update the comment - or type of a fileset, set a property and value pair for a fileset, or remove a property from a fileset. + """A fileset change is a change to a fileset. It can be used to rename a fileset, + update the comment of a fileset, update the type of a fileset, set a property and + value pair for a fileset, or remove a property from a fileset. """ @staticmethod From 3ead2aee6e4822a4dbe6017e9ba436d787d4d38a Mon Sep 17 00:00:00 2001 From: mchades Date: Thu, 22 Oct 2026 16:32:11 +0800 Subject: [PATCH 3/3] fix(fileset): Clarify type conversion permissions and lifecycle --- .../authorization/FilesetAuthorizationIT.java | 50 +++++++++++++++++++ .../gravitino/api/file/fileset_change.py | 8 ++- docs/filesets.md | 8 +-- ...manage-fileset-metadata-using-gravitino.md | 8 +++ 4 files changed, 70 insertions(+), 4 deletions(-) diff --git a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/FilesetAuthorizationIT.java b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/FilesetAuthorizationIT.java index a651dda5148..76a58917cc9 100644 --- a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/FilesetAuthorizationIT.java +++ b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/FilesetAuthorizationIT.java @@ -438,6 +438,56 @@ public void testUseSecretsPrivilegeForGetSecrets() { adminFilesetCatalog.dropFileset(filesetIdent); } + /** Verifies that writers can change the fileset lifecycle without gaining drop permission. */ + @Test + @Order(9) + public void testWriterCanConvertFilesetTypeButCannotDrop() { + String filesetName = "type_conversion_fileset"; + NameIdentifier filesetIdent = NameIdentifier.of(SCHEMA, filesetName); + MetadataObject filesetObject = + MetadataObjects.of( + ImmutableList.of(CATALOG, SCHEMA, filesetName), MetadataObject.Type.FILESET); + GravitinoMetalake metalake = client.loadMetalake(METALAKE); + FilesetCatalog ownerFilesetCatalog = metalake.loadCatalog(CATALOG).asFilesetCatalog(); + FilesetCatalog writerFilesetCatalog = + normalUserClient.loadMetalake(METALAKE).loadCatalog(CATALOG).asFilesetCatalog(); + + ownerFilesetCatalog.createFileset( + filesetIdent, + "comment", + Fileset.Type.EXTERNAL, + storageLocation(GravitinoITUtils.genRandomName("FilesetAuthorizationIT_type_conversion")), + new HashMap<>()); + try { + metalake.grantPrivilegesToRole( + role, filesetObject, ImmutableList.of(Privileges.ReadFileset.allow())); + assertThrows( + ForbiddenException.class, + () -> + writerFilesetCatalog.alterFileset( + filesetIdent, FilesetChange.updateType(Fileset.Type.MANAGED))); + assertEquals(Fileset.Type.EXTERNAL, ownerFilesetCatalog.loadFileset(filesetIdent).type()); + + metalake.grantPrivilegesToRole( + role, filesetObject, ImmutableList.of(Privileges.WriteFileset.allow())); + Fileset managedFileset = + writerFilesetCatalog.alterFileset( + filesetIdent, FilesetChange.updateType(Fileset.Type.MANAGED)); + assertEquals(Fileset.Type.MANAGED, managedFileset.type()); + assertThrows(ForbiddenException.class, () -> writerFilesetCatalog.dropFileset(filesetIdent)); + assertEquals(Fileset.Type.MANAGED, writerFilesetCatalog.loadFileset(filesetIdent).type()); + + Fileset externalFileset = + writerFilesetCatalog.alterFileset( + filesetIdent, FilesetChange.updateType(Fileset.Type.EXTERNAL)); + assertEquals(Fileset.Type.EXTERNAL, externalFileset.type()); + assertThrows(ForbiddenException.class, () -> writerFilesetCatalog.dropFileset(filesetIdent)); + assertEquals(Fileset.Type.EXTERNAL, writerFilesetCatalog.loadFileset(filesetIdent).type()); + } finally { + ownerFilesetCatalog.dropFileset(filesetIdent); + } + } + private String defaultBaseLocation() { if (defaultBaseLocation == null) { defaultBaseLocation = diff --git a/clients/client-python/gravitino/api/file/fileset_change.py b/clients/client-python/gravitino/api/file/fileset_change.py index 1bb65859f1e..89c8a947ce9 100644 --- a/clients/client-python/gravitino/api/file/fileset_change.py +++ b/clients/client-python/gravitino/api/file/fileset_change.py @@ -216,7 +216,7 @@ def __str__(self): class UpdateFilesetType: """A fileset change to update the type without changing data or locations.""" - _new_type: Fileset.Type = field(metadata=config(field_name="newType")) + _new_type: Fileset.Type = field(metadata=config(field_name="new_type")) def __post_init__(self): if not isinstance(self._new_type, Fileset.Type): @@ -226,6 +226,12 @@ def new_type(self) -> Fileset.Type: """Returns the new fileset type.""" return self._new_type + def __eq__(self, other) -> bool: + """Compares fileset type changes by their target type.""" + if not isinstance(other, FilesetChange.UpdateFilesetType): + return False + return self._new_type == other.new_type() + def __hash__(self): return hash(self._new_type) diff --git a/docs/filesets.md b/docs/filesets.md index ae160222b90..bc486613712 100644 --- a/docs/filesets.md +++ b/docs/filesets.md @@ -58,14 +58,16 @@ See [How to use GVFS](./how-to-use-gvfs.md). A managed fileset belongs to Gravitino. Creating it creates the directory if needed, and deleting it deletes the data. -An external fileset points at a location that already exists and stays under someone else's control. -Deleting it removes the Gravitino record and leaves the files alone. +An external fileset leaves data lifecycle management to the user. Deleting it removes only the +Gravitino record and preserves the files. With filesystem operations enabled, creating either +type creates the storage directory if it does not exist. An existing fileset can be converted between the two types using [`FilesetChange.updateType`](./manage-fileset-metadata-using-gravitino.md#change-the-fileset-type). Conversion changes metadata only and preserves its identity, locations, and files. Converting to external relinquishes management of the data; converting to managed accepts deletion of the data -when the fileset is dropped. +when the fileset is dropped. With `disable-filesystem-ops=true`, dropping a fileset removes only +its metadata, even after conversion to `MANAGED`. ### Storage Locations diff --git a/docs/manage-fileset-metadata-using-gravitino.md b/docs/manage-fileset-metadata-using-gravitino.md index d66a1764943..10ab4996255 100755 --- a/docs/manage-fileset-metadata-using-gravitino.md +++ b/docs/manage-fileset-metadata-using-gravitino.md @@ -196,6 +196,14 @@ deleting that data when the fileset is dropped. The same type semantics apply wh schema with cascade. Conversion does not move, create, or delete files, and does not protect data from deletion through another managed fileset with an overlapping location. +With `disable-filesystem-ops=true`, dropping a fileset removes only its metadata, even after +conversion to `MANAGED`. + +Type conversion follows the existing [fileset alter authorization](security/access-control.md#data-objects). +A caller with `WRITE_FILESET` and the required catalog and schema use privileges can convert in +either direction, including changing whether a subsequent drop deletes the data. This does not +grant permission to drop the fileset; drop retains its existing ownership requirements. +