Skip to content

Flink: Fix equalityFieldColumns always null in IcebergSink#14952

Merged
pvary merged 2 commits into
apache:mainfrom
Guosmilesmile:equality_fix
Jan 6, 2026
Merged

Flink: Fix equalityFieldColumns always null in IcebergSink#14952
pvary merged 2 commits into
apache:mainfrom
Guosmilesmile:equality_fix

Conversation

@Guosmilesmile

Copy link
Copy Markdown
Contributor

Currently, equalityFieldColumns in IcebergSink is always null, because although the builder holds equalityFieldColumns and converts it to equalityFieldColumnIds before passing it to IcebergSink, the raw equalityFieldColumns list is only used in log statements. Hence, no issue has surfaced so far.

This PR fixes the issue where equalityFieldColumns is always null, ensuring that the parameter can be correctly printed in the logs when problems occur.

@github-actions github-actions Bot added the flink label Jan 2, 2026

@singhpk234 singhpk234 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM thanks @Guosmilesmile !

@huaxingao huaxingao left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM


private final Table table;
private final Set<String> equalityFieldColumns = null;
private final Set<String> equalityFieldColumns;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could we leave a comment that this should only be used for logging, and equalityFieldIds should be used instead?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, have added a comment for it.

@pvary pvary merged commit 42cac92 into apache:main Jan 6, 2026
20 checks passed
@pvary

pvary commented Jan 6, 2026

Copy link
Copy Markdown
Contributor

Merged to main.
Thanks @Guosmilesmile for the PR and @singhpk234, @huaxingao and @ebyhr for the reviews!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants