Skip to content

Conversation

@snuyanzin
Copy link
Contributor

What is the purpose of the change

Part of FLIP-550
The PR to add support for operations like

-- add single column
ALTER MATERIALIZED TABLE myMaterializedTable ADD c1 AS current_timestamp COMMENT 'new_column docs';

-- add multiple columns, constraint, and watermark
ALTER MATERIALIZED TABLE myMaterializedTable ADD (
     ts AS current_timestamp FIRST,
     col_meta INT METADATA FROM 'mk1' VIRTUAL AFTER col_b,
     PRIMARY KEY (id) NOT ENFORCED,
     WATERMARK FOR ts AS ts - INTERVAL '3' SECOND
);

Brief change log

converters and tests

Verifying this change

there are tests added

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): (no)
  • The public API, i.e., is any changed class annotated with @Public(Evolving): (no)
  • The serializers: (no)
  • The runtime per-record code paths (performance sensitive): (no)
  • Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: (no)
  • The S3 file system connector: (no)

Documentation

  • Does this pull request introduce a new feature? (yes)
  • If yes, how is the feature documented? (JavaDocs, also will be done separately)

@flinkbot
Copy link
Collaborator

flinkbot commented Dec 2, 2025

CI report:

Bot commands The @flinkbot bot supports the following commands:
  • @flinkbot run azure re-run the last Azure build

Copy link
Contributor

@raminqaf raminqaf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution @snuyanzin! Excellent PR, with many improvements and cleanups! Left some nit comments. Should be good for the merge in the next round!

Copy link
Contributor

@twalthr twalthr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @snuyanzin. I added a couple of minor comments to make code a bit more readability and improve error messages. Overall the PR looks great!

+ "FRESHNESS = INTERVAL '30' SECOND\n"
+ "REFRESH_MODE = FULL\n"
+ "AS SELECT * FROM t3",
"Column 'c' referenced by materialized table option 'partition.fields.c.date-formatter' isn't a partition column. Available partition columns: ['']."),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[''] bug?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, here in SQL there is a table option partition.fields.c.date-formatter referencing column c, however at the same time there is no any partition defined for the materialized table and as a result no available partition columns

+ "FROM `builtin`.`default`.`t1` AS `t1`");
}

private static class TestSpec {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you for setting up more test infra!

Copy link
Contributor

@raminqaf raminqaf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution! LGTM!

@github-actions github-actions bot added the community-reviewed PR has been reviewed by the community. label Dec 3, 2025
@snuyanzin snuyanzin force-pushed the flink38766 branch 3 times, most recently from d20fe7b to 7555eb4 Compare December 3, 2025 23:39
Copy link
Contributor

@twalthr twalthr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the update @snuyanzin. I just had some minor comments. Maybe you can address them before merging. But overall the PR looks good to me.

snuyanzin added a commit to snuyanzin/flink that referenced this pull request Dec 4, 2025
@snuyanzin snuyanzin closed this in fc85065 Dec 4, 2025
Samrat002 pushed a commit to Samrat002/flink that referenced this pull request Dec 5, 2025
@snuyanzin snuyanzin deleted the flink38766 branch December 7, 2025 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community-reviewed PR has been reviewed by the community.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants