Skip to content

Flink: Preserve RowKind in DynamicIcebergSink's DataConverter (cherry-pick apache/iceberg#18101) - #13

Merged
nickwu241 merged 2 commits into
1.11.0-affirm-patchfrom
nick/cherry-pick-18101-rowkind-dynamic-sink
Sep 25, 2026
Merged

nickwu241 merged 2 commits into
1.11.0-affirm-patchfrom
nick/cherry-pick-18101-rowkind-dynamic-sink

Conversation

@nickwu241

@nickwu241 nickwu241 commented Sep 25, 2026 •

Copy link
Copy Markdown
Member

What

RowDataConverter.convert() rebuilds a row via new GenericRowData(fieldGetters.length) whenever its schema doesn't structurally match the target table's (missing column, widened type, different order). GenericRowData's no-arg-kind constructor defaults RowKind to INSERT, and nothing downstream restores the source row's real kind.

Why this matters for us

replication-kt's CdcDynamicRecordGenerator emits RowKind.DELETE for CDC deletes, and ValidatedStreamWriterApp/CdcLogWriterApp/TxnMetadataLogWriterApp all set .immediateTableUpdate(true) (eager schema-evolution commit). A CDC delete carrying an older schema than a just-evolved target table hits this exact rebuild path and gets silently coerced into an INSERT. Pre-existing on master/1.10.2 today tracked as apache#18100.

Fix

new GenericRowData(fieldGetters.length) → new GenericRowData(sourceData.getRowKind(), fieldGetters.length).

RowDataConverter.convert() rebuilt a row as new GenericRowData(fieldGetters.length)
whenever its schema didn't structurally match the target table's, which defaults
RowKind to INSERT and silently drops the source row's actual kind. A CDC
delete/UPDATE_BEFORE routed through this rebuild path was coerced into an INSERT.

Cherry-picked from apache#18101 (closes apache#18100).
v1.20/v2.1 are covered by apache#18196; v2.0 was never covered upstream
(the flink/v2.0 module was dropped from apache/iceberg main by apache#17849
before apache#18196 landed), so this is a manual port rather than a real cherry-pick.

@dcagney dcagney left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Approving — one-line fix matches upstream apache#18101 exactly, byte-for-byte across flink/v1.20, v2.0, v2.1.

Verified the changelog's second citation, apache#18196 (upstream's own backport of this fix to v1.20/v2.1/v2.2) — our extension to v2.0 is correctly scoped since upstream has since dropped that module from main, so this isn't a discrepancy.

Checked specifically for hidden dependency on unbackported upstream work: our fork's pre-patch DataConverter.java matches upstream's pre-fix state exactly across all three modules, and GenericRowData(RowKind, int) is a plain Flink API constructor with no Iceberg-side prerequisite. No missing dependency.

LGTM.

@nickwu241
nickwu241 merged commit d04e323 into 1.11.0-affirm-patch Sep 25, 2026
45 of 57 checks passed
nickwu241 added a commit that referenced this pull request Sep 25, 2026
Bumps version.txt and adds the AFFIRM_VERSIONS.md changelog entry for the next
patch release, covering the 3 review-triggered fixes from all-the-things#222721
(#12/#13/#14). Those PRs are open, not yet merged — rebase this branch onto
1.11.0-affirm-patch once they land, then tag 1.11.0-affirm-patch.2 at the
resulting commit and publish per RUNBOOK_ICEBERG_PATCH_PUBLISH.md.
nickwu241 added a commit that referenced this pull request Sep 25, 2026
* Document and stage 1.11.0-affirm-patch.2

Bumps version.txt and adds the AFFIRM_VERSIONS.md changelog entry for the next
patch release, covering the 3 review-triggered fixes from all-the-things#222721
(#12/#13/#14). Those PRs are open, not yet merged — rebase this branch onto
1.11.0-affirm-patch once they land, then tag 1.11.0-affirm-patch.2 at the
resulting commit and publish per RUNBOOK_ICEBERG_PATCH_PUBLISH.md.

* Update AFFIRM_VERSIONS.md: base off 1.11.0-affirm-patch.1, drop reviewer names

* Drop #10/#11 from .2 table (already covered by .1 base)

* Note #12/#13/#14 merged and branch rebased; ready to tag

* Drop 'not yet published' framing now that .2 contents are final

* Drop trailing note from .2 section to match .1 format
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.

Flink: DynamicIcebergSink writes CDC delete records as inserts when the record needs schema conversion

2 participants