[feature] Support alter database comment and custom properties by LiebingYu · Pull Request #1172 · apache/fluss

LiebingYu · 2025-06-21T08:03:50Z

Purpose

Linked issue: close #1151

Brief change log

Tests

API and Format

Documentation

Copilot

Pull request overview

This PR adds end-to-end support for ALTER DATABASE to update a database’s comment and custom properties across the RPC API, server coordinator, Java client admin, and Flink catalog integration (linked to #1151).

Changes:

Introduces a new RPC API (ALTER_DATABASE) with request/response messages and gateway wiring.
Implements server-side handling to apply database property changes and persist them in ZooKeeper.
Adds client- and Flink-side implementations plus new unit/integration tests for altering database properties/comments.

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
fluss-server/src/test/java/org/apache/fluss/server/coordinator/TestCoordinatorGateway.java	Extends test gateway interface with `alterDatabase` stub.
fluss-server/src/main/java/org/apache/fluss/server/zk/ZooKeeperClient.java	Adds ZK helper to update an existing database znode payload.
fluss-server/src/main/java/org/apache/fluss/server/utils/ServerRpcMessageUtils.java	Adds PbAlterConfig → `DatabaseChange` conversions (including comment special-casing).
fluss-server/src/main/java/org/apache/fluss/server/entity/DatabasePropertyChanges.java	New internal representation of database property/comment changes.
fluss-server/src/main/java/org/apache/fluss/server/coordinator/MetadataManager.java	Adds coordinator logic to apply and persist database changes.
fluss-server/src/main/java/org/apache/fluss/server/coordinator/CoordinatorService.java	Adds `alterDatabase` RPC handler + conversion to `DatabasePropertyChanges`.
fluss-rpc/src/main/proto/FlussApi.proto	Defines `AlterDatabaseRequest/Response` messages.
fluss-rpc/src/main/java/org/apache/fluss/rpc/protocol/ApiKeys.java	Adds new `ALTER_DATABASE` API key.
fluss-rpc/src/main/java/org/apache/fluss/rpc/gateway/AdminGateway.java	Exposes `alterDatabase` on the admin RPC gateway.
fluss-flink/fluss-flink-common/src/test/java/org/apache/fluss/flink/sink/testutils/TestAdminAdapter.java	Updates test adapter interface surface with `alterDatabase` stub.
fluss-flink/fluss-flink-common/src/test/java/org/apache/fluss/flink/catalog/FlinkCatalogTest.java	Adds catalog-level unit coverage for altering database props/comments.
fluss-flink/fluss-flink-common/src/test/java/org/apache/fluss/flink/catalog/FlinkCatalogITCase.java	Adds SQL-based IT coverage for `ALTER DATABASE ... SET (...)`.
fluss-flink/fluss-flink-common/src/main/java/org/apache/fluss/flink/catalog/FlinkCatalog.java	Implements Flink `Catalog#alterDatabase` by diffing descriptors into `DatabaseChange`s.
fluss-common/src/main/java/org/apache/fluss/metadata/DatabaseChange.java	New public change model for database alters (set/reset/updateComment).
fluss-common/src/main/java/org/apache/fluss/config/FlussConfigUtils.java	Adds shared constant `COMMENT_PROP_NAME = "comment"`.
fluss-client/src/test/java/org/apache/fluss/client/admin/FlussAdminITCase.java	Adds IT coverage for admin-side alter database flows.
fluss-client/src/main/java/org/apache/fluss/client/utils/ClientRpcMessageUtils.java	Adds `DatabaseChange` → PbAlterConfig conversion for database alters.
fluss-client/src/main/java/org/apache/fluss/client/admin/FlussAdmin.java	Implements `Admin#alterDatabase` RPC call construction and dispatch.
fluss-client/src/main/java/org/apache/fluss/client/admin/Admin.java	Adds new public `alterDatabase` API to the Admin interface.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-27T02:10:17Z

fluss-server/src/main/java/org/apache/fluss/server/coordinator/MetadataManager.java

+                // Update the database in ZooKeeper
+                DatabaseRegistration updatedRegistration = DatabaseRegistration.of(newDescriptor);


alterDatabaseProperties builds DatabaseRegistration via DatabaseRegistration.of(newDescriptor), which sets both createdTime and modifiedTime to System.currentTimeMillis(). This will overwrite the database's original create time on every ALTER. Consider constructing the updated registration using the existing createdTime from the current DB registration/info and only bumping modifiedTime (similar to how TableRegistration#newProperties preserves createdTime).

Suggested change

// Update the database in ZooKeeper

DatabaseRegistration updatedRegistration = DatabaseRegistration.of(newDescriptor);

// Preserve original createdTime and only update modifiedTime

long createdTime = databaseInfo.getCreatedTime();

long modifiedTime = System.currentTimeMillis();

DatabaseRegistration updatedRegistration =

new DatabaseRegistration(newDescriptor, createdTime, modifiedTime);

Copilot · 2026-02-27T02:10:17Z

fluss-server/src/main/java/org/apache/fluss/server/coordinator/MetadataManager.java

+        newCustomProperties.keySet().removeAll(changes.customPropertiesToReset);
+
+        boolean hasCommentChange =
+                !currentDescriptor.getComment().equals(Optional.ofNullable(changes.commentToSet));


hasCommentChange is derived from changes.commentToSet, but commentToSet defaults to null. That means an ALTER that only changes custom properties (or even an empty change list) will be interpreted as a comment change and will clear any existing comment. The change model needs a tri-state (no-op vs set comment vs reset comment), e.g., a boolean commentTouched flag in DatabasePropertyChanges that's only set when a comment change is explicitly requested.

Suggested change

!currentDescriptor.getComment().equals(Optional.ofNullable(changes.commentToSet));

changes.commentToSet != null

&& !currentDescriptor

.getComment()

.equals(Optional.of(changes.commentToSet));

Copilot · 2026-02-27T02:10:17Z

fluss-server/src/main/java/org/apache/fluss/server/entity/DatabasePropertyChanges.java

+        private String commentToSet = null;
+


commentToSet defaults to null, but null is also the desired value for a legitimate "reset comment" operation. As written, DatabasePropertyChanges cannot represent "no comment change" vs "reset comment", which leads to unintended comment clearing when only properties are altered. Consider adding an explicit flag (e.g., commentChanged/commentTouched) or storing the comment change as an Optional wrapper distinct from the value itself.

Copilot · 2026-02-27T02:10:18Z

fluss-server/src/main/java/org/apache/fluss/server/entity/DatabasePropertyChanges.java

+            customPropertiesToReset.add(key);
+        }
+
+        public void setComment(String comment) {


Builder#setComment accepts a non-null String, but it is called with updateComment.getComment() which can be null (to reset the comment). Please update the method signature/annotation to accept @Nullable and (ideally) also record that the comment was explicitly changed so null can mean "reset" rather than "not provided".

Suggested change

public void setComment(String comment) {

public void setComment(@Nullable String comment) {

Copilot · 2026-02-27T02:10:18Z

fluss-common/src/main/java/org/apache/fluss/metadata/DatabaseChange.java

+
+        @Override
+        public String toString() {
+            return "SetComment{" + "comment='" + comment + '\'' + '}';


UpdateComment#toString() returns the prefix SetComment{...} which doesn't match the class name / operation (UpdateComment). This makes logs/debug output confusing; consider updating the string to reflect UpdateComment (and include whether it is clearing vs setting).

Suggested change

return "SetComment{" + "comment='" + comment + '\'' + '}';

return "UpdateComment{"

+ (comment == null ? "clearComment" : "comment='" + comment + '\'')

+ '}';

loserwang1024

Thanks for your job, I have left some comment.

loserwang1024 · 2026-02-27T02:21:35Z

fluss-flink/fluss-flink-common/src/main/java/org/apache/fluss/flink/catalog/FlinkCatalog.java

+        } catch (Exception e) {
+            Throwable t = ExceptionUtils.stripExecutionException(e);
+            if (CatalogExceptionUtils.isDatabaseNotExist(t)) {
+                if (!ignoreIfNotExists) {


admin.alterDatabase(databaseName, databaseChanges, ignoreIfNotExists) already handle ignoreIfNotExists. Thus, just throw exception here

loserwang1024 · 2026-02-27T02:26:22Z

fluss-rpc/src/main/proto/FlussApi.proto

 }

+// alter database request and response
+message AlterDatabaseRequest {


where is comment? I think comment should be seperated from config_changes, @wuchong , WDYT?

polyzos force-pushed the main branch 3 times, most recently from d88c76c to 434a4f4 Compare August 31, 2025 15:13

LiebingYu force-pushed the alter_database branch 2 times, most recently from 98ba457 to c3292f4 Compare October 27, 2025 06:05

luoyuxia force-pushed the main branch from 0e2d71f to 74ee888 Compare February 6, 2026 12:32

[server] Support alter database comment and custom properties

ae84bc6

LiebingYu force-pushed the alter_database branch from c3292f4 to ae84bc6 Compare February 26, 2026 11:20

luoyuxia requested review from Copilot and loserwang1024 February 27, 2026 01:59

Copilot started reviewing on behalf of luoyuxia February 27, 2026 01:59 View session

Copilot AI reviewed Feb 27, 2026

View reviewed changes

loserwang1024 reviewed Feb 27, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[feature] Support alter database comment and custom properties#1172

[feature] Support alter database comment and custom properties#1172
LiebingYu wants to merge 1 commit intoapache:mainfrom
LiebingYu:alter_database

LiebingYu commented Jun 21, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 27, 2026

Uh oh!

Copilot AI Feb 27, 2026

Uh oh!

Copilot AI Feb 27, 2026

Uh oh!

Copilot AI Feb 27, 2026

Uh oh!

Copilot AI Feb 27, 2026

Uh oh!

loserwang1024 left a comment

Uh oh!

loserwang1024 Feb 27, 2026

Uh oh!

loserwang1024 Feb 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		// Update the database in ZooKeeper
		DatabaseRegistration updatedRegistration = DatabaseRegistration.of(newDescriptor);

-                // Update the database in ZooKeeper
-                DatabaseRegistration updatedRegistration = DatabaseRegistration.of(newDescriptor);
+                // Preserve original createdTime and only update modifiedTime
+                long createdTime = databaseInfo.getCreatedTime();
+                long modifiedTime = System.currentTimeMillis();
+                DatabaseRegistration updatedRegistration =
+                        new DatabaseRegistration(newDescriptor, createdTime, modifiedTime);

-                !currentDescriptor.getComment().equals(Optional.ofNullable(changes.commentToSet));
+                changes.commentToSet != null
+                        && !currentDescriptor
+                                .getComment()
+                                .equals(Optional.of(changes.commentToSet));

	public void setComment(String comment) {
	public void setComment(@Nullable String comment) {

-            return "SetComment{" + "comment='" + comment + '\'' + '}';
+            return "UpdateComment{"
+                    + (comment == null ? "clearComment" : "comment='" + comment + '\'')
+                    + '}';

Conversation

LiebingYu commented Jun 21, 2025

Purpose

Brief change log

Tests

API and Format

Documentation

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

loserwang1024 left a comment

Choose a reason for hiding this comment

Uh oh!

loserwang1024 Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

loserwang1024 Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants