Skip to content

Flink: Fix NPE in ExpireSnapshots config when retain-last is unset#17277

Draft
thswlsqls wants to merge 1 commit into
apache:mainfrom
thswlsqls:fix/flink-expire-snapshots-retain-last-npe
Draft

Flink: Fix NPE in ExpireSnapshots config when retain-last is unset#17277
thswlsqls wants to merge 1 commit into
apache:mainfrom
thswlsqls:fix/flink-expire-snapshots-retain-last-npe

Conversation

@thswlsqls

Copy link
Copy Markdown
Contributor

Closes #17275

Summary

  • ExpireSnapshots.Builder.config() passed the nullable ExpireSnapshotsConfig.retainLast() (Integer) to retainLast(int); unboxing threw NullPointerException at ExpireSnapshots.java:119.
  • retain-last has no default — RETAIN_LAST_OPTION is .intType().noDefaultValue() and no table property backs it. TestExpireSnapshotsConfig already asserts it is null by default.
  • Not an edge case: IcebergSink.java:813 calls config() unconditionally once flink-maintenance.expire-snapshots.enabled is on.
  • The setter now runs only when the value is non-null, leaving numSnapshots (already a nullable Integer) unset otherwise. Same shape as the sibling ConvertEqualityDeletes.Builder.config() (ConvertEqualityDeletes.java:136-144), which guards its nullable scheduleOnIntervalSecond().
  • Optional.ofNullable(...).orElse(null), used above for maxSnapshotAge, cannot work here: retainLast takes a primitive, so orElse(null) unboxes and throws anyway.
  • Signatures unchanged; behavior when retain-last is set is identical.
Integer retainLast = expireSnapshotsConfig.retainLast();
if (retainLast != null) {
  retainLast(retainLast);
}

Testing done

  • Added TestExpireSnapshotsConfig#configureBuilderWithoutRetainLast, feeding a default config into ExpireSnapshots.builder().config(...). Fails with the NPE above without the fix, passes with it.
  • :iceberg-flink:iceberg-flink-2.1:test over TestExpireSnapshotsConfig, TestExpireSnapshots, TestConvertEqualityDeletesConfig — 10 tests, 0 failures.
  • spotlessCheck, checkstyleMain, checkstyleTest passed. Module-wide :check not run to completion locally (many MiniClusters); left to CI.

ExpireSnapshots.Builder.config() passed the nullable
ExpireSnapshotsConfig.retainLast() to retainLast(int), which unboxed
null and threw. The retain-last option has no default and is not backed
by a table property, so an unset value is the normal state and enabling
expire-snapshots maintenance without it always failed.

Call the setter only when the value is present, leaving the builder's
numSnapshots unset otherwise. This mirrors
ConvertEqualityDeletes.Builder.config(), which already guards its
nullable scheduleOnIntervalSecond().

Generated-by: Claude Code
@github-actions github-actions Bot added the flink label Jul 17, 2026
@thswlsqls
thswlsqls marked this pull request as draft July 17, 2026 11:16
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: NPE in ExpireSnapshots config when retain-last is unset

1 participant