Skip to content

[FLINK-39495][table] Fix FROM_CHANGELOG silently dropping rows with unmapped operation codes#27973

Merged
twalthr merged 3 commits intoapache:masterfrom
raminqaf:FLINK-39495
Apr 21, 2026
Merged

[FLINK-39495][table] Fix FROM_CHANGELOG silently dropping rows with unmapped operation codes#27973
twalthr merged 3 commits intoapache:masterfrom
raminqaf:FLINK-39495

Conversation

@raminqaf
Copy link
Copy Markdown
Contributor

@raminqaf raminqaf commented Apr 20, 2026

What is the purpose of the change

FROM_CHANGELOG silently drops input rows when the operation code column contains a value not present in the op_mapping (or not matching the default
mapping). This makes data loss invisible and very hard to debug — users have no indication that rows are being discarded.

This PR changes the default behavior to throw a TableRuntimeException when an unmapped operation code is encountered, making data issues visible immediately
rather than causing silent data loss.

Brief change log

  • Changed FromChangelogFunction to throw a TableRuntimeException instead of silently returning when an unmapped op code is encountered

  • Removed the UNMAPPED_CODES_DROPPED semantic test that asserted silent-drop behavior

  • Removed a duplicate input type strategy test ("Valid with UPDATE_BEFORE" was identical to "Valid with custom mapping")

  • Added ChangelogFunctionITCase with a runtime test that verifies the exception is thrown for unmapped op codes

    Verifying this change

    This change added tests and can be verified as follows:

  • Added ChangelogFunctionITCase.testFromChangelogFailsOnUnmappedOpCode — sets up a source with an "UNKNOWN" op code and asserts that FROM_CHANGELOG
    throws a TableRuntimeException containing the invalid op code and the defined op codes

    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): yes (adds a branch in the per-record eval path, but only on the error case which was
    previously a silent return)

  • 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? no

  • If yes, how is the feature documented? not applicable


Was generative AI tooling used to co-author this PR?
  • Yes (please specify the tool below)
    Claude Code (Claude Opus 4.7)

@flinkbot
Copy link
Copy Markdown
Collaborator

flinkbot commented Apr 20, 2026

CI report:

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

@github-actions github-actions Bot added the community-reviewed PR has been reviewed by the community. label Apr 20, 2026
throw new TableRuntimeException(
String.format(
"Received invalid op code '%s'. Defined op codes are: %s.",
opCode, opMap.keySet()));
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.

Please double check if the current document from_changelog has been synchronized and modified

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.

Updated the docs with the new behavior

@@ -137,7 +138,10 @@ public void eval(
final StringData opCode = input.getString(opColumnIndex);
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.

Is it possible for opCode to be null at present

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.

It's an important point. We can classify this as an invalid op code and and fail/skip as configured by the user https://cwiki.apache.org/confluence/display/FLINK/FLIP-564%3A+Support+FROM_CHANGELOG+and+TO_CHANGELOG+built-in+PTFs#431-invalid-operation-codes

Copy link
Copy Markdown
Contributor Author

@raminqaf raminqaf Apr 21, 2026

Choose a reason for hiding this comment

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

For now we go with default behavior, which would be failing during runtime.

…ed op codes

Reflect the behavior change from silent drop to TableRuntimeException in the
FROM_CHANGELOG documentation: update the op_mapping description and add an
explicit note after the default mapping table.
Throw TableRuntimeException when an input row carries a NULL op code instead
of silently producing an INSERT. Document the behavior in the op parameter
description and add a NULL_OP_CODE semantic test.
Copy link
Copy Markdown
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.

LGTM

Copy link
Copy Markdown
Contributor

@gustavodemorais gustavodemorais 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 updates, @raminqaf. lgtm!

@twalthr twalthr merged commit b12302d into apache:master Apr 21, 2026
twalthr pushed a commit that referenced this pull request Apr 22, 2026
…nmapped operation codes

This closes #27973.

(cherry picked from commit b12302d)
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.

5 participants