Skip to content

feat(ftindex): read Java full-text archives via paimon-ftindex-core#563

Merged
JingsongLi merged 9 commits into
apache:mainfrom
JunRuiLee:feat/ftindex-core-foundation
Jul 22, 2026
Merged

feat(ftindex): read Java full-text archives via paimon-ftindex-core#563
JingsongLi merged 9 commits into
apache:mainfrom
JunRuiLee:feat/ftindex-core-foundation

Conversation

@JunRuiLee

@JunRuiLee JunRuiLee commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Purpose

First step of mirroring Java Paimon's primary-key full-text search into the Rust read side. Java's full-text index is backed by a shared native Rust core, paimon-ftindex-core (public repo apache/paimon-full-text, tantivy 0.26.1), which the Java/Python bindings wrap via JNI/ctypes. This PR lets the Rust reader depend on that same core directly (no JNI needed), so it can read the on-disk archive format Java writes.

This is a deliberately small, self-contained foundation slice: dependency wiring + a thin reader wrapper + round-trip tests. It de-risks the external dependency (does it fetch and build here?) before any of the primary-key scan/read machinery is built on top.

Brief change log

  • Add optional dependency paimon-ftindex-core = "0.1.0" (crates.io), wired into the existing fulltext cargo feature via dep: syntax.
  • New feature-gated module crates/paimon/src/ftindex/ with FullTextArchiveReader<R: SeekRead>:
    • from_seek_read(R) — open over any SeekRead implementation, so large/remote archives can be range-read without whole-file buffering (used by the append/global-index path in feat(fulltext): read global indexes via paimon-ftindex-core #571).
    • from_input_file(&InputFile) — whole-file convenience wrapper; holds the read Bytes directly via a BytesReader (no second copy), so peak memory stays at roughly one archive.
    • search(query_json, limit) — JSON-DSL query, returns FullTextHits { row_ids, scores }.
    • search_with_include(query_json, limit, &RoaringTreemap) — restricts results to a live-row allow-list (the withIncludeRowIds equivalent); borrows the bitmap so callers reusing an allow-list need not clone it, and an empty allow-list short-circuits to no hits.
  • Re-export the core I/O types used by the public API (SeekRead, ReadRequest, SliceReader) plus BytesReader, FullTextArchiveReader, FullTextHits from paimon::ftindex, so consumers can implement SeekRead or name the reader while depending only on paimon.
  • Fail-loud error mapping (engine errors surface as crate::Error with the underlying source preserved, no silent fallback).

Out of scope (later PRs): shared PK-index-layer generalization, CoreOptions full-text options, PK full-text scan/read, hybrid-on-PK, and migrating the existing append/data-evolution full-text path off raw tantivy 0.22 onto the core.

Tests

  • ftindex::reader::tests::test_round_trip_search_returns_expected_rows — build an archive with the core writer, read it back, assert the correct row-ids/scores.
  • ftindex::reader::tests::test_search_with_include_restricts_to_allow_list — all docs match; the roaring include-filter restricts results to the allow-list subset (also validates cross-crate roaring 0.11 compatibility).
  • ftindex::reader::tests::test_search_with_include_empty_or_disjoint_returns_no_hits — an empty or disjoint allow-list admits no rows.
  • ftindex::reader::tests::test_from_seek_read_opens_archive_directly — exercises the generic from_seek_read constructor directly (the API the remote-FileIO path in feat(fulltext): read global indexes via paimon-ftindex-core #571 depends on).
  • Full cargo test -p paimon --features fulltext green; cargo clippy -p paimon --lib --tests --features fulltext -- -D warnings clean; default (no-feature) build, minimal fulltext + storage-memory build, cargo package -p paimon, and cargo build -p paimon-datafusion (Send boundary) all pass.

API and Format

paimon-ftindex-core is now depended on from crates.io (paimon-ftindex-core = "0.1.0"), so there is no git dependency, cargo package -p paimon succeeds, and this PR is ready to merge (no longer draft).

No public API change to existing paths and no storage-format change. Adds a new feature-gated reader over the existing Java-written full-text archive format (v1), read through the shared paimon-ftindex-core engine. The fulltext feature transitionally pulls both tantivy 0.22 (legacy append path) and 0.26 (via the core); the duplicate is removed when the append path converges onto the core in a later PR.

Documentation

No documentation changes; module-level doc comments describe the new reader.

@shyjsarah

Copy link
Copy Markdown
Contributor

I’ve completed the implementation and am currently waiting for the paimon-full-text release. Once it’s available, I’ll submit the PR. I’d really appreciate your review then.

@JunRuiLee

JunRuiLee commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

I’ve completed the implementation and am currently waiting for the paimon-full-text release. Once it’s available, I’ll submit the PR. I’d really appreciate your review then.

Thanks for the heads-up! Could you share which part you've been working on?
I'd originally planned to do the full primary-key full-text and hybrid read path myself, and my implementation is already quite far along — I've opened #568 as a complete draft so you can see what it covers (reader foundation → PK full-text data layer → PK full-text read path → PK hybrid search), all passing the full test suite; it's just gated on the paimon-ftindex-core crates.io release before I can mark it ready.

Tracking issue: #567. You're very welcome to review once it's up.

This PR (and #568) is only waiting on the paimon-ftindex-core crates.io release — the dependency is pinned to a GitHub git tag for now, and as soon as the crate is published I'll switch that to the crates.io version and mark the PRs ready. You're very welcome to review once they're up.

@shyjsarah

shyjsarah commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Thanks for sharing #568. I've opened #571 as a draft so the scope and implementation are visible.

My work focuses on the existing non-primary-key/global-index full-text path and DataFusion integration, rather than the primary-key full-text and hybrid search path covered by #568. It migrates the legacy in-repository Tantivy implementation to paimon-ftindex-core and covers snapshot index-manifest reads, fast/full/detail modes, deletion vectors, and raw fallback.

There is some overlap with #563 in the dependency and reader wiring. I'm happy to rebase on or reuse that foundation to avoid duplication. Please let me know which integration order you would prefer. I'll also review #568 separately.

@JunRuiLee

Copy link
Copy Markdown
Contributor Author

Thanks for sharing #568. I've opened #571 as a draft so the scope and implementation are visible.

My work focuses on the existing non-primary-key/global-index full-text path and DataFusion integration, rather than the primary-key full-text and hybrid search path covered by #568. It migrates the legacy in-repository Tantivy implementation to paimon-ftindex-core and covers snapshot index-manifest reads, fast/full/detail modes, deletion vectors, and raw fallback.

There is some overlap with #563 in the dependency and reader wiring. I'm happy to rebase on or reuse that foundation to avoid duplication. Please let me know which integration order you would prefer. I'll also review #568 separately.

Thanks — the scope is clear now, and I think our work is complementary with essentially no functional overlap: you're migrating the append/global-index full-text path (and DataFusion) onto paimon-ftindex-core, and #568 covers the primary-key full-text + hybrid path.

The only real intersection is the shared piece: the paimon-ftindex-core dependency + the FullTextArchiveReader wiring (currently in #563). I'd suggest we land that reader foundation first, as its own small PR (#563), and then both of our stacks — your global-index path and my PK path — rebase onto it so the dependency and reader live in one place. That also lets us settle one shared detail: the pinned tag (I'm on v0.1.0-rc4, you're on v0.1.0-rc5) — we should converge on one and switch to the crates.io version before merging, since a git-only dependency blocks publishing.

Works for me to have #563 go in first; happy to adjust the reader's API if your global path needs anything from it.

@shyjsarah

Copy link
Copy Markdown
Contributor

Thanks, that ordering works for me.

One API requirement from #571: global-index archives may be stored on remote FileIO and can be large, so the current implementation intentionally avoids reading the entire archive into memory. It adapts range reads from FileRead to paimon_ftindex_core::io::SeekRead with bounded concurrency.

Could #563 make FullTextArchiveReader generic over SeekRead, or provide a from_seek_read constructor while keeping from_input_file as a convenience API? That would let both the global-index and PK paths reuse the same reader foundation without forcing whole-file reads.

I'll keep #571 as a draft and rebase it onto #563 once we agree on this API and the foundation lands.

@JunRuiLee

JunRuiLee commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

Done — I've updated #563 with a generic SeekRead API (from_seek_read constructor + FullTextArchiveReader<R: SeekRead + 'static>). The from_input_file convenience wrapper is preserved for whole-file reads. You can now rebase #571 onto the updated foundation and use AsyncFileSeekRead with it. Let me know if the API needs any adjustment.

Thanks, that ordering works for me.

One API requirement from #571: global-index archives may be stored on remote FileIO and can be large, so the current implementation intentionally avoids reading the entire archive into memory. It adapts range reads from FileRead to paimon_ftindex_core::io::SeekRead with bounded concurrency.

Could #563 make FullTextArchiveReader generic over SeekRead, or provide a from_seek_read constructor while keeping from_input_file as a convenience API? That would let both the global-index and PK paths reuse the same reader foundation without forcing whole-file reads.

I'll keep #571 as a draft and rebase it onto #563 once we agree on this API and the foundation lands.

Done — I've updated #563 with a generic SeekRead API (from_seek_read constructor + FullTextArchiveReader<R: SeekRead + 'static>). The from_input_file convenience wrapper is preserved for whole-file reads. You can now rebase #571 onto the updated foundation and use AsyncFileSeekRead with it. Let me know if the API needs any adjustment.

@shyjsarah

Copy link
Copy Markdown
Contributor

Thanks for the quick update! I tested #571 on top of the latest #563 and integrated AsyncFileSeekRead with FullTextArchiveReader::from_seek_read. It works well for the global-index path, and all relevant tests and Clippy checks pass. No further API changes are needed from my side. I’ll keep #571 as a draft and rebase it once #563 is merged.

Allow both whole-file (SliceReader) and streaming/range-read strategies
for full-text archive access. Adds from_seek_read constructor; keeps
from_input_file as a convenience wrapper for the whole-file case.

Addresses coordination with global-index full-text path.
@JunRuiLee
JunRuiLee force-pushed the feat/ftindex-core-foundation branch from cce3f55 to dcf88ba Compare July 22, 2026 06:45
@JunRuiLee
JunRuiLee marked this pull request as ready for review July 22, 2026 06:46
Switch from the temporary git tag to the published 0.1.0 release now that
the crate is on crates.io, so `cargo package`/publishing of paimon works.
@JunRuiLee
JunRuiLee force-pushed the feat/ftindex-core-foundation branch from dcf88ba to 217b73d Compare July 22, 2026 07:07

@JingsongLi JingsongLi left a comment

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.

Thanks for adding this shared full-text reader foundation. I reviewed the implementation against paimon-ftindex-core 0.1.0, the existing Tantivy path, and the expected consumers in the PK and global-index read paths.

The reader behavior looks correct, and I did not find a functional search or archive-compatibility issue. I also verified the following locally:

  • The two new ftindex::reader tests pass.
  • The minimal fulltext + storage-memory feature build passes.
  • Clippy with the fulltext feature passes with warnings denied.
  • cargo package -p paimon succeeds.
  • All GitHub CI checks are green.

However, I recommend addressing the following API issues before approval.

1. Re-export the core I/O types used by the public API

FullTextArchiveReader<R> and from_seek_read expose the paimon_ftindex_core::io::SeekRead trait in Paimon's public API. The concrete reader returned by from_input_file also contains the core's SliceReader type. However, these types are not re-exported from paimon::ftindex.

As a result, an external consumer cannot implement from_seek_read or explicitly name the in-memory reader type while depending only on paimon; it must add its own direct dependency on paimon-ftindex-core. This partially defeats the goal of keeping the shared reader and dependency wiring in one foundation module, and it also couples consumers to the exact core dependency version selected by Paimon.

Could we either:

  • re-export SeekRead, ReadRequest, and SliceReader from paimon::ftindex; or
  • hide the core types behind Paimon-owned public traits, adapters, or type aliases?

For example, the module could expose something similar to:

pub use paimon_ftindex_core::io::{ReadRequest, SeekRead, SliceReader};
pub use reader::{FullTextArchiveReader, FullTextHits};

Please also add a direct unit test for FullTextArchiveReader::from_seek_read. This is the key API required by the remote FileIO integration in #571, while the current tests only exercise from_input_file.

2. Avoid consuming the include bitmap

search_with_include currently takes RoaringTreemap by value:

pub fn search_with_include(
    &self,
    query_json: &str,
    limit: usize,
    include_row_ids: RoaringTreemap,
)

The method only serializes the bitmap and does not need ownership. Taking it by value forces callers that reuse an allow-list across multiple archive searches to clone potentially large bitmaps. The planned PK consumer already needs to call include.clone() for this reason.

Please consider accepting &RoaringTreemap instead:

pub fn search_with_include(
    &self,
    query_json: &str,
    limit: usize,
    include_row_ids: &RoaringTreemap,
)

It would also be useful to return an empty FullTextHits immediately when the include bitmap is empty, instead of executing the full query and filtering every matching document out inside the collector.

3. Correct the memory claim in from_input_file

The documentation currently says that the whole-file implementation keeps peak memory at one archive:

a whole-read keeps peak memory at one archive

However, the implementation first receives a Bytes buffer and then creates a second complete buffer with bytes.to_vec():

let bytes = input.read().await?;
Self::from_seek_read(SliceReader::new(bytes.to_vec()))

During the conversion, both buffers coexist, so the temporary peak archive memory is approximately twice the archive size. This may matter for large PK full-text archives if the convenience constructor is used there.

Could we either:

  • adjust the documentation so it does not claim one-archive peak memory; or
  • provide a Bytes-backed SeekRead implementation to avoid the extra full-file copy?

The generic from_seek_read path correctly allows large remote archives to avoid whole-file loading, so this is mainly about making the convenience API and its documentation accurate.

4. Update the PR description

The PR description is stale after the latest commit. It still says:

  • the dependency is a Git tag such as v0.1.0-rc4;
  • the crate is not available on crates.io;
  • the git dependency blocks packaging; and
  • the PR should remain Draft until the release.

The current code uses the released registry dependency:

paimon-ftindex-core = { version = "0.1.0", optional = true }

The PR is also no longer Draft, and cargo package succeeds. Please update the description so that the final PR history accurately reflects the implementation being merged.

Other than these API and documentation points, the implementation and dependency integration look good.

Map FtIndexError and the roaring row-id filter serialization error into UnexpectedError { source: Some(Box::new(e)) } instead of discarding the underlying error, matching the vindex/lumina convention so the engine error chain survives.

Add a regression test asserting an empty or disjoint include-set admits no rows (not all rows), locking in the search_with_include filter semantics.
- Re-export the core I/O types the public API exposes (SeekRead, ReadRequest, SliceReader) plus BytesReader/FullTextArchiveReader/FullTextHits from paimon::ftindex, so consumers can implement SeekRead or name the reader without depending on paimon-ftindex-core directly. Add a direct from_seek_read unit test.
- search_with_include now borrows &RoaringTreemap (it only serializes the bitmap) and short-circuits to empty hits when the allow-list is empty, sparing callers a clone and a wasted query.
- Add a Bytes-backed BytesReader and use it in from_input_file so a whole-file read is not copied a second time via SliceReader's Vec<u8>; correct the peak-memory doc accordingly.
@JunRuiLee
JunRuiLee force-pushed the feat/ftindex-core-foundation branch from 8b12a6b to 2cc6c68 Compare July 22, 2026 11:29
@JunRuiLee

Copy link
Copy Markdown
Contributor Author

Thanks @JingsongLi for the thorough review and for verifying the build/package/CI locally. All four points are addressed in 2cc6c68:

1. Re-export the core I/O typespaimon::ftindex now re-exports SeekRead, ReadRequest, and SliceReader (plus FullTextArchiveReader, FullTextHits, and the new BytesReader), so a consumer can implement SeekRead or name the reader while depending only on paimon. Added test_from_seek_read_opens_archive_directly, which exercises the generic constructor directly (bypassing from_input_file) — the entry point #571's remote-FileIO path uses.

2. Avoid consuming the include bitmapsearch_with_include now takes &RoaringTreemap; it only serializes the bitmap, so callers reusing an allow-list across archives no longer clone. It also returns an empty FullTextHits immediately when the allow-list is empty, skipping the query entirely.

3. Memory claim in from_input_file — went with the stronger fix rather than only correcting the doc: added a Bytes-backed BytesReader and switched from_input_file to hold the Bytes from input.read() directly, so there is no longer a second to_vec() buffer — peak is genuinely ~one archive now. Doc updated to match, and from_seek_read remains the path for large/remote archives that should not be fully buffered.

4. PR description — updated to reflect the crates.io 0.1.0 dependency, the generic SeekRead API + re-exports, the ready (non-draft) state, and that cargo package -p paimon succeeds.

Verification after the change: cargo test -p paimon --features fulltext (the four ftindex::reader tests + full suite), cargo clippy -p paimon --lib --tests --features fulltext -- -D warnings, default build, minimal fulltext + storage-memory build, cargo package -p paimon, and cargo build -p paimon-datafusion all pass.

@JunRuiLee JunRuiLee closed this Jul 22, 2026
@JunRuiLee JunRuiLee reopened this Jul 22, 2026

@JingsongLi JingsongLi left a comment

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.

+1

@JingsongLi
JingsongLi merged commit f8471ea into apache:main Jul 22, 2026
25 of 26 checks passed
@JunRuiLee
JunRuiLee deleted the feat/ftindex-core-foundation branch July 22, 2026 14:15
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.

3 participants