Thanks for the update!
Looks good to me.
Best,
Jingsong
On Thu, Nov 24, 2022 at 3:07 PM Shengkai Fang <fskm...@gmail.com> wrote:
>
> Hi, Jingsong.
>
> Thanks for your feedback!
>
> > Can we define classes at once so that the connector can be fully
> implemented, but we will not pass changes of the nested column.
>
> It's hard to achieve this. If a new need comes, we will add a new kind of
> TableChange. But I think the current proposal aligns with the Flink syntax.
>
> > In addition, can we define the modified class of the nested column as the
> parent class? The top-level column modification is just a special case of a
> subclass.
>
> I think the modification of the composite type is much more complicated. It
> also modifies the MAP, ARRAY type. For example, Spark supports to modify
> the element of ARRAY type with the following syntax:
>
>
> ```
> -- add a field to the struct within an array. Using keyword 'element' to
> access the array's element column.
> ALTER TABLE prod.db.sample
> ADD COLUMN points.element.z double
> ```
>
> For the traditional database, they use the ALTER TYPE syntax to modify the
> composite type.
>
> ```
> CREATE TYPE compfoo AS (f1 int, f2 text);
>
> -- add column
> ALTER TYPE compfoo ADD ATTRIBUTE f3 int;
> -- rename column
> ALTER TYPE compfoo RENAME ATTRIBUTE f3 TO f4;
> -- drop column
> ALTER TYPE compfoo DROP ATTRIBUTE f4;
> -- modify type
> ALTER TYPE compfoo ALTER ATTRIBUTE f1 TYPE text;
>
> ```
>
> I think the modification of the top-level column is different from the
> modification of the nested field, and we don't need to introduce a base
> class. BTW, the RexFieldAccess is also not the parent class of the
> RexInputRef in Calcite.
>
> > ModifyColumn VS fine-grained Change
>
> The syntax in Flink also supports modifying the physical column to a
> metadata column, metadata column definition, etc. The main problem here is
> we need to expose what kind of changes happen when modifying the metadata
> column, modifying the computed column, and changing the column kind. With
> these possibilities, it may include:
>
> base: ModifyColumnComment, MoidfyColumnPosition;
> physical column: ModifyPhysicalColumnDataType,
> ModifyPhysicalColumnNullability;
> metadata column: ModifyMetadataColumnMetadataKey,
> ModifyMetadataColumnVirtual, ModifyMetdataColumnDataType;
> computed column: ModifyComputedColumnExpression
> column type change: ModifyPhysicalColumnToMetadataColumn,
> ModifyPhysicalColumnToComputedColumn, ModifyComputedColumnToPhysicalColumn,
> ModifyComputedColumnToMetadataColumn,
> ModifyMetadataColumnToPhysicalColumn, ModifyMetadataColumnToComputedColumn.
>
> I just wonder whether it's better we still keep the ModifyColumn and
> introduce some fine-grained TableChanges because the most cases above are
> not useful to external catalog, e.g. JDBCCatalog. So I think we can
> introduce the ModifyColumn that represents column modification and
>
> ```
> /** Modify the column comment */
> public class ModifyColumnComment extends ModifyColumn {
>
> String getComment();
>
> }
>
> /** Modify the column position. */
> public class ModifyColumnPosition extends ModifyColumn {}
>
> /** Modify the physical column data type. */
> public class ModifyPhysicalColumnType extends ModifyColumn {
>
> DataType getNewType();
>
> }
>
> ```
>
> When altering the physical column, the statement will produce the
> fine-grained changes. In other cases, we will still produce the base class
> ModifyColumn.
>
>
> > ModifyColumn can also rename the column.
>
> Yes. I have modified the FLIP and moved the RenameColumn and added a class
> named ModifyColumnName extends ModifyColumn.
>
> Best,
> Shengkai
