Skip to content

feat(bindings/java): add OperatorReader and ReaderOptions - #8259

Open
jihuayu wants to merge 24 commits into
apache:mainfrom
jihuayu:codex/8252
Open

jihuayu wants to merge 24 commits into
apache:mainfrom
jihuayu:codex/8252

Conversation

@jihuayu

@jihuayu jihuayu commented Sep 8, 2026

Copy link
Copy Markdown
Member

Which issue does this PR close?

Closes #8252.

Rationale for this change

Expose the OperatorReader and ReaderOptions abstractions from the Rust core in the Java binding.

What changes are included in this PR?

  • Add Operator.createReader(...), OperatorReader, and ReaderOptions, allowing a reader to be configured and then used either for independent range reads or to create an input stream. gap is used by Reader::fetch to merge multiple ranges. Since Java does not currently expose this API, this option is not included.

  • Rename the internal Reader used by OperatorInputStream to NativeIterator, and update the corresponding JNI names accordingly, to distinguish it from the newly introduced OperatorReader. This type and its related native methods are private implementation details, so the rename does not affect user code.

  • Explicitly throw IllegalStateException when reading from a closed stream, and synchronize close() with read() to prevent the native iterator from being released while a read is in progress, which could otherwise result in invalid memory access.

Are there any user-facing changes?

Yes. Callers can use OperatorReader Now.

Breaking changes

None

AI Usage Statement

  • Harness: Codex desktop. Model: GPT-6. For code implementation and validation.

@jihuayu
jihuayu requested a review from tisonkun as a code owner September 8, 2026 08:50
* @return a stream that the caller must close
* @throws OpenDALException if reader options are invalid (ConfigInvalid) or creation fails
*/
public OperatorInputStream createInputStream(String path, ReadOptions readOptions, ReaderOptions readerOptions) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ReadOptions overlaps ReaderOptions. It seems we should just accept ReaderOptions here?

@jihuayu jihuayu Sep 14, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Hi @Xuanwo

I see. Yeah, this abstraction isn’t quite right.

I looked more closely into the Java binding, the Rust core, and the history of these APIs.

It seems that Java's current ReadOptions is a legacy binding-specific abstraction introduced before RFC-6213, rather than a direct counterpart of the core ReadOptions. This makes the current read/stream API a bit awkward once ReaderOptions is introduced.

The parameter here should be ReaderOptions, but it is currently named ReadOptions. I need to think about how to correct the API semantics while preserving backward compatibility.

@jihuayu jihuayu changed the title feat(java): expose core reader tuning options in Java bindings feat(java): add reusable readers with reader options Sep 15, 2026
@jihuayu jihuayu changed the title feat(java): add reusable readers with reader options feat(bindings/java): add OperatorReader and ReaderOptions Sep 15, 2026
@jihuayu

jihuayu commented Sep 15, 2026

Copy link
Copy Markdown
Member Author

Hi @Xuanwo. I finally figured out this code abstraction, and I hope this is what you had in mind.

In this PR, we introduce ReaderOptions. After that, I’ll start working on migrating ReadOptions.

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.

new feature: expose core reader tuning options in Java bindings

2 participants