Skip to content

Add 4-arg map override to FilterFileChannel#15801

Merged
mayya-sharipova merged 1 commit intoapache:mainfrom
mayya-sharipova:filterchannel-map-arena
Mar 9, 2026
Merged

Add 4-arg map override to FilterFileChannel#15801
mayya-sharipova merged 1 commit intoapache:mainfrom
mayya-sharipova:filterchannel-map-arena

Conversation

@mayya-sharipova
Copy link
Contributor

Java 22 added FileChannel.map(MapMode, long, long, Arena) which
returns MemorySegment. The base FileChannel class provides a default
implementation that throws UnsupportedOperationException. Without
this override, any wrapped FileChannel based on FilterFileChannel
would fail when the 4-arg map variant is called.

Java 22 added FileChannel.map(MapMode, long, long, Arena) which
returns MemorySegment. The base FileChannel class provides a default
implementation that throws UnsupportedOperationException. Without
this override, any wrapped FileChannel based on FilterFileChannel
would fail when the 4-arg map variant is called.
@mayya-sharipova
Copy link
Contributor Author

@ldematte would appreciate your review here

@mayya-sharipova mayya-sharipova added the skip-changelog Apply to PRs that don't need a changelog entry, stopping the automated changelog check. label Mar 6, 2026
Copy link

@ldematte ldematte left a comment

Choose a reason for hiding this comment

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

The change looks good, it's the missing overload. However I'm not sure about one thing: this API is JDK 22+, and AFAIK lucene supports JDK 21. This could fail to compile (or at least give you warnings) with target 21, but most importantly it will fail at runtime if we run this with JDK 21. Again, not sure if this is an issue or not, especially since this is test code, but I wanted to point it out.

@dweiss
Copy link
Contributor

dweiss commented Mar 9, 2026

AFAIK lucene supports JDK 21

Not on main, no. Main is jdk25+.

@ldematte
Copy link

ldematte commented Mar 9, 2026

Not on main, no. Main is jdk25+.

Oh nice! So I don't see any issue with this change.

@benwtrent
Copy link
Member

Not on main, no. Main is jdk25+.

Yep, this just cannot be backported to 10x.

@mayya-sharipova mayya-sharipova merged commit f7b35cb into apache:main Mar 9, 2026
12 of 13 checks passed
@uschindler
Copy link
Contributor

Hi,
as the test-only file system provider now allows this (wasn't possible with Java 21 due to public API differences and preview state), the foillowing line should also be removed:

// Work around for JDK-8259028: we need to unwrap our test-only file system layers
path = Unwrappable.unwrapAll(path);

This is no longer needed because the test-wrappers now behave correctly.

@uschindler
Copy link
Contributor

Oh this was already resolved, can you do the change on main branch or should I?

uschindler added a commit to uschindler/lucene that referenced this pull request Mar 9, 2026
@uschindler
Copy link
Contributor

Here is the PR: #15804

Will merge when tests are done.

uschindler added a commit that referenced this pull request Mar 9, 2026
@uschindler
Copy link
Contributor

Done.

@uschindler
Copy link
Contributor

Basically this was my fault as I did not remove the MMapDirectory unwrapping after update to Java 22+ on main (see #14564, especially https://github.com/apache/lucene/pull/14564/changes#diff-8681a7b2259f78a6d0d2efe4894ef79ce4697c3825d24675248bc557617773ceL58). If I would have removed it I would have noticed the missing override anyways. Note to myself: "When migrating code to later java versions check your TODOs and workarounds in source code!"

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

Labels

module:test-framework skip-changelog Apply to PRs that don't need a changelog entry, stopping the automated changelog check.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants