Skip to content

[core] fix RenamingSnapshotCommitTest compile error from #6778#7942

Closed
TheR1sing3un wants to merge 1 commit into
apache:masterfrom
TheR1sing3un:fix/renaming-snapshot-commit-test-compile
Closed

[core] fix RenamingSnapshotCommitTest compile error from #6778#7942
TheR1sing3un wants to merge 1 commit into
apache:masterfrom
TheR1sing3un:fix/renaming-snapshot-commit-test-compile

Conversation

@TheR1sing3un
Copy link
Copy Markdown
Member

@TheR1sing3un TheR1sing3un commented May 24, 2026

Problem

After #6778 landed, paimon-core no longer compiles on master:

paimon-core/src/test/java/org/apache/paimon/catalog/RenamingSnapshotCommitTest.java:[124,17]
  incompatible types: possible lossy conversion from long to int

createSnapshot() builds the Snapshot via:

Map<Integer, Long> logOffsets = new HashMap<>();
...
return new Snapshot(
        id, schemaId, baseManifestList, baseManifestListSize, ...,
        timeMillis,
        logOffsets,          // ← stray 14th arg
        totalRecordCount, deltaRecordCount,
        null, null, null, null, null);

Snapshot (paimon-api/src/main/java/org/apache/paimon/Snapshot.java) has two public constructors — 20 args and 22 args — and neither contains a logOffsets parameter. With the extra arg the call is 21-wide; javac picks the closest 22-arg overload and the type system then reports a misleading "long → int" mismatch on the first arg.

Every PR opened against master right now hits this.

Fix

Drop the stray logOffsets declaration and the corresponding constructor argument (plus the two now-unused java.util.Map / java.util.HashMap imports). The remaining 20 args line up positionally with the 20-arg Snapshot ctor.

This assumes logOffsets was an accidental leftover — Snapshot itself is not intended to carry a per-bucket log-offset map. If the intent of #6778 was actually to extend Snapshot with a logOffsets field, that's a larger change that should land in Snapshot.java (with serialization, equals/hashCode, etc.); flag this PR and we can switch direction.

Verification

CI on this branch is the source of truth — local Maven was not run. The change is mechanical (remove a never-referenced local + the matching arg + two imports) and the arg counts are spelled out above.

The new createSnapshot() helper in RenamingSnapshotCommitTest passes
a stray Map<Integer, Long> logOffsets argument as the 14th parameter
of `new Snapshot(...)`. The Snapshot constructor has no such field —
its public ctors are 20-arg and 22-arg, neither contains logOffsets —
so javac picks the nearest overload and the test source no longer
compiles ("possible lossy conversion from long to int" at line 124).

Remove the logOffsets local plus the corresponding constructor arg
(and the two now-unused imports). The remaining 20 arguments line up
with the 20-arg public Snapshot constructor.
@TheR1sing3un
Copy link
Copy Markdown
Member Author

@JingsongLi HI, please review it! Let's fix it! Thanks!

@TheR1sing3un
Copy link
Copy Markdown
Member Author

The one failing build job is a transient artifact-download timeout while resolving org.aliyun.lumina:lumina-jni:0.2.0 from lumina-binary.oss-cn-shanghai.aliyuncs.com — unrelated to this change. paimon-core itself compiled cleanly (the build progressed all the way through paimon-lance before the paimon-lumina dependency resolution timed out). A retry of the workflow should clear it.

Copy link
Copy Markdown
Contributor

@JingsongLi JingsongLi left a comment

Choose a reason for hiding this comment

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

LGTM. The fix is straightforward — removing the stale logOffsets arg and unused imports so the 20-arg Snapshot ctor matches again.

One minor note: if there's an upcoming plan to add logOffsets back to Snapshot (with proper serialization), we should track that separately. But as a compile-fix this is correct.

@JingsongLi
Copy link
Copy Markdown
Contributor

Fixed in master.

@JingsongLi JingsongLi closed this May 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants