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