Thanks for the discussion and sorry for jumping in late.

Hi, Yuxia,
As we introduced in the java-client documentation[1], the Admin API manages
metadata and the cluster, the Table API is used for **reading and writing**
tables.
This will be a clear separation for Admin and Table, so I think it's fine
to put the `alterTable` in Admin.

Hi Yang,
Here are some my comments:
(1) As we want to upgrade to proto3 syntax some time in the future, so we
try to avoid using `enum` type in the proto which is incompatible.
(2) I suggest to only introduce a `AlterTableConfigsRequest`, this allow us
can easily introduce a independent alter schema RPC in the future.
(3) The alter config message can be resued with FIP-12

The RPC can look like this

message AlterTableConfigsRequest {
  required PbTablePath table_path = 1;
  required bool ignore_if_not_exists = 2;
  repeated PbAlterConfigsRequestInfo config_changes = 3;
}

// we have introduced this PB message in FIP-12
message PbAlterConfigsRequestInfo{
  required string config_key = 1;
  optional string config_value = 2;
  required int32 op_type = 3; // SET=0, DELETE=1, APPEND=2, SUBTRACT=3
}

message AlterTablePropertiesResponse {
}

What do you think?

Best,
Jark

[1]: https://fluss.apache.org/docs/apis/java-client/
[2]:
https://github.com/apache/fluss/blob/master/fluss-rpc/src/main/proto/FlussApi.proto#L27


On Tue, 16 Sept 2025 at 21:59, Yang Guo <[email protected]> wrote:

> Hi,
>
> Thanks for feedback from yuxia and liebing.
>
> I compared the implementation on update properties. Here is my
> understanding:
>
> In our design, to alter table, we need to provide two kinds of info:
> `TablePath` and `FlussTabledChanges`. The `TablePath` provides the table
> information and the `FlussTabledChanges` defines the changes. They are
> independent to each other.
> But in iceberg, the `UpdateProperties` defines both the table information
> and table changes. Each `UpdateProperties` object has the whole
> information needed to alter a table. e.g:
> ```
> class PropertiesUpdate implements UpdateProperties {
>   private final TableOperations ops;
>   private final Map<String, String> updates = Maps.newHashMap();
>   private final Set<String> removals = Sets.newHashSet();
>   private TableMetadata base;
> }
> ```
>
> The difference is that, in the iceberg, `UpdateProperties` implements an
> interface called `PendingUpdate`. To put all info together, it allows the
> update not be executed immediately. We can control when to `apply` and
> `commit` those changes.
> ```
> public interface PendingUpdate<T> {
>
>   /**
>    * Apply the pending changes and return the uncommitted changes for
> validation.
>    */
>   T apply();
>
>   /**
>    * Apply the pending changes and commit.
>    *
>    * <p>Changes are committed by calling the underlying table's commit
> method.
>    *
>    * <p>Once the commit is successful, the updated table will be refreshed.
>    */
>   void commit();
> }
> ```
>
> Back to Fluss, I think it does not have any differences in clients side, to
> pass `TablePath` and `FlussTableChanges` independently or together.
> But in Fluss server side, if we need those 'PendingUpdate' features, we may
> have a new class to put them together. e.g:
> ```
> class TablePathWithChanges {
>     TablePath tablePath;
>     List<FlussTableChanges> tableChanges;
> }
> ```
>
> And for the class name, I will change from `FlussTableChange` to
> `TableChange` after we settle down the design.
>
> Regards,
> Yang Guo
>
>
>
> On Mon, Sep 15, 2025 at 2:31 PM Liebing Yu <[email protected]> wrote:
>
> > Hi, Yang Guo!
> >
> > Thanks for moving this forward. For the interfaces, I suggest using
> > TableChange instead of FlussTableChange. Since we are always in Fluss,
> > adding a "Fluss" prefix is a bit redundant. Another reason is that we
> might
> > also use this interface when modifying lake tables, in which case we are
> > not modifying Fluss tables.
> >
> > Best regards,
> > Liebing Yu
> >
> >
> > On Fri, 12 Sept 2025 at 15:26, yuxia <[email protected]>
> wrote:
> >
> > > Hi, Yang Guo!
> > > Thnaks for driving this FIP. It must be a good improvement.
> > >
> > > About Api design in client side. I want to raise the design of iceberg
> > for
> > > update properties[1] into discussion.
> > > The difference is how the api is attached to. In iceberg, the api is
> > > attached to `Table` instance instead of something like `Admin` or
> > > `Catalog`.
> > > Here is an example of How iceberg does:
> > > ```
> > > public interface Table {
> > >
> > >   /**
> > >    * Create a new {@link UpdateProperties} to update table properties
> and
> > > commit the changes.
> > >    *
> > >    * @return a new {@link UpdateProperties}
> > >    */
> > >   UpdateProperties updateProperties();
> > >
> > > }
> > > ```
> > > ```
> > > public interface UpdateProperties {
> > >   /**
> > >    * Add a key/value property to the table.
> > >    *
> > >    * @param key a String key
> > >    * @param value a String value
> > >    * @return this for method chaining
> > >    * @throws NullPointerException If either the key or value is null
> > >    */
> > >   UpdateProperties set(String key, String value);
> > >
> > >   /**
> > >    * Remove the given property key from the table.
> > >    *
> > >    * @param key a String key
> > >    * @return this for method chaining
> > >    * @throws NullPointerException If the key is null
> > >    */
> > >   UpdateProperties remove(String key);
> > > }
> > > ```
> > >
> > > I don't have a strong opinion on this, but following Iceberg's design
> > > approach raises another question: how should we define the
> > responsibilities
> > > of `Admin` versus `Table`?
> > >
> > > For example, methods like listOffsets and getLatestKvSnapshots are
> > > currently placed in `Admin`, but they could also be appropriate for
> > > `Table`. This makes the responsibility boundaries less clear.
> > >
> > > cc @Jark What's your opinion on it.
> > >
> > >
> > > [1]
> > >
> >
> https://github.com/apache/iceberg/blob/main/api/src/main/java/org/apache/iceberg/Table.java#L191
> > >
> > >
> > >
> > > Best regards,
> > > Yuxia
> > >
> > > ----- 原始邮件 -----
> > > 发件人: "Yang Guo" <[email protected]>
> > > 收件人: "dev" <[email protected]>
> > > 发送时间: 星期四, 2025年 9 月 11日 下午 9:33:38
> > > 主题: [DISCUSS] FIP-15: Alter Table Interface
> > >
> > > Hi all,
> > >
> > > Fluss server and client do not support alter tables now, but there are
> > many
> > > features required to alter table properties. After discussion with
> other
> > > fluss developers, I'd like to draft this FIP about the alter table
> > > interface.
> > >
> > > This FIP-15 is to design an alter table interface. Briefly speaking it
> > adds
> > > an 'alterTable' API in both client and server side. Considering there
> > will
> > > be various different types of changes on a table (such as comments,
> > > properties, buckets, columns, etc), we introduce a 'FlussTableChange'
> > class
> > > to distinguish them. It will also make servers easy to determine how to
> > > operate on the table based on change types.
> > >
> > > For mode details, you can see the FIP-15 draft:
> > >
> > >
> >
> https://cwiki.apache.org/confluence/display/FLUSS/FIP-15%3A+Alter+Table+Interface
> > >
> > >
> > > I also have implemented a PR to alter table properties based on this
> > > design, which could be a sample for this design:
> > > https://github.com/apache/fluss/pull/1625
> > >
> > > Thanks in advance and looking forward to any feedback from you.
> > >
> > > Regards,
> > > Yang Guo
> > >
> >
>

Reply via email to