Skip to content

feat: expose cached file metadata APIs on FileFragment#7820

Merged
BubbleCal merged 1 commit into
lance-format:mainfrom
jiaqizho:make-get-file-metadata-pub
Jul 17, 2026
Merged

feat: expose cached file metadata APIs on FileFragment#7820
BubbleCal merged 1 commit into
lance-format:mainfrom
jiaqizho:make-get-file-metadata-pub

Conversation

@jiaqizho

@jiaqizho jiaqizho commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Make the full and indexed file metadata lookup methods public so callers can reuse the fragment's dataset-level metadata cache instead of loading metadata independently through FileReader.

I ran into this while trying to inspect the file footer of a fragment that had already been opened. The obvious option was to call FileReader::read_all_metadata, but that API reads directly from storage and does not reuse the dataset’s file metadata cache. This can result in another metadata I/O even though the fragment has already loaded the same information.

FileFragment already has cache-aware helpers for loading the full metadata or metadata index. This change makes get_file_metadata and get_file_metadata_index public so other fragment-level operations can reuse the existing cached metadata instead of reading it again through FileReader.

Summary by CodeRabbit

  • New Features
    • Exposed file metadata and metadata index access for broader integration and tooling.

Make the full and indexed file metadata lookup methods public so callers can
reuse the fragment's dataset-level metadata cache instead of loading metadata
independently through FileReader.
@github-actions github-actions Bot added the enhancement New feature or request label Jul 16, 2026
@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

FileFragment::get_file_metadata and get_file_metadata_index are now public asynchronous methods. Their parameters, implementations, and control flow remain unchanged.

Changes

FileFragment Metadata Access

Layer / File(s) Summary
Expose metadata accessors
rust/lance/src/dataset/fragment.rs
get_file_metadata and get_file_metadata_index now use public visibility without other implementation changes.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Suggested reviewers: xuanwo, wjones127

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the change: it exposes cached file metadata APIs on FileFragment.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot 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.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
rust/lance/src/dataset/fragment.rs (3)

1623-1637: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Complete the rustdoc for both newly public APIs.

get_file_metadata lacks an example and links to its related types, while get_file_metadata_index has no rustdoc at all. Document the scheduler/file relationship, cache behavior, known_schema, and return types with examples matching the actual signatures.

As per coding guidelines, public Rust APIs must include examples and links to relevant structs and methods.

Also applies to: 1640-1663

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@rust/lance/src/dataset/fragment.rs` around lines 1623 - 1637, The newly
public methods get_file_metadata and get_file_metadata_index need complete
rustdoc. Add documentation describing the FileScheduler relationship, metadata
caching, known_schema behavior, and each method’s actual return type, with links
to relevant structs and methods and examples that compile against their
signatures. Keep the implementation unchanged.

Source: Coding guidelines


1623-1663: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Add regression coverage for the public accessors.

Add tests that invoke both methods through their public API and verify full/indexed metadata retrieval, including reuse of the dataset metadata cache.

As per coding guidelines, every feature must have corresponding tests.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@rust/lance/src/dataset/fragment.rs` around lines 1623 - 1663, Add regression
tests for the public get_file_metadata and get_file_metadata_index accessors,
covering complete and indexed metadata retrieval and confirming repeated calls
reuse the dataset metadata cache. Use the existing fragment/dataset test
utilities and exercise both methods through their public API, including the
known_schema path where applicable.

Source: Coding guidelines


1623-1628: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Guard the file-scheduler path before caching metadata. Both helpers trust file_scheduler.reader().path() and will read/cache metadata for whatever handle they receive, even if it is not one of self.metadata.files. Add a fragment-path check or make the precondition explicit in the API docs.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@rust/lance/src/dataset/fragment.rs` around lines 1623 - 1628, Update
get_file_metadata and the related metadata helper to validate that
file_scheduler.reader().path() belongs to self.metadata.files before reading or
caching metadata. Reject handles for paths outside the fragment, or explicitly
document this as a required API precondition if validation is intentionally
omitted; ensure the cache is never accessed for an unrelated path.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@rust/lance/src/dataset/fragment.rs`:
- Around line 1623-1637: The newly public methods get_file_metadata and
get_file_metadata_index need complete rustdoc. Add documentation describing the
FileScheduler relationship, metadata caching, known_schema behavior, and each
method’s actual return type, with links to relevant structs and methods and
examples that compile against their signatures. Keep the implementation
unchanged.
- Around line 1623-1663: Add regression tests for the public get_file_metadata
and get_file_metadata_index accessors, covering complete and indexed metadata
retrieval and confirming repeated calls reuse the dataset metadata cache. Use
the existing fragment/dataset test utilities and exercise both methods through
their public API, including the known_schema path where applicable.
- Around line 1623-1628: Update get_file_metadata and the related metadata
helper to validate that file_scheduler.reader().path() belongs to
self.metadata.files before reading or caching metadata. Reject handles for paths
outside the fragment, or explicitly document this as a required API precondition
if validation is intentionally omitted; ensure the cache is never accessed for
an unrelated path.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 09dd1f4a-e5dc-4823-ac05-7ddc4b35ef8d

📥 Commits

Reviewing files that changed from the base of the PR and between 4f90cf4 and 62a09a1.

📒 Files selected for processing (1)
  • rust/lance/src/dataset/fragment.rs

jiaqizho added a commit to jiaqizho/milvus-storage that referenced this pull request Jul 16, 2026
Lance row groups currently report memory_size as a placeholder value of 1, which gives cache and scheduling callers no useful idea of how much decoded memory a chunk may occupy. We need a best-effort estimate that also works for externally referenced Lance files, without relying on extra information being recorded when the file is written.

This change derives the estimate from the existing schema and footer metadata. Fixed-width leaf columns use the logical row count multiplied by their value width. Variable-width and nested columns use Lance page metadata: completed pages follow the 8 MiB writer page target, the final page is prorated from its row count, and encoded page bytes are only used as a lower bound or single-page fallback instead of being mistaken for decoded memory. Deletions are handled by applying the live-to-physical row ratio once, and physical columns belonging to nested fields are grouped back into their top-level schema columns.

The Rust bridge now provides estimates for each top-level column and keeps the fragment-level API as the saturating sum of those values. LanceTableReader distributes the fragment estimate across logical chunks according to their row ranges, so the chunk values add back up to the fragment total and remain consistent across projections and cached reader creation. Estimation stays best effort on the C++ side, with a zero fallback when metadata cannot be estimated, including unsupported legacy files.

The estimator resolves both normal dataset files and files referenced through external base paths. It only reads schema, column, and page metadata from the footer and never reads or decompresses data pages. It also relies on Lance's normalized ColumnInfo and PageInfo representation for storage formats 2.1 through 2.3, avoiding encoding-specific logic that would need to be maintained whenever the file format changes. The current Lance version does not expose get_file_metadata publicly, so the implementation temporarily uses the public read_all_metadata path without sharing Lance's metadata cache; after upgrading to a version containing lance-format/lance#7820, this can switch to cached metadata and remove the duplicate footer I/O.

Signed-off-by: jiaqizho <[email protected]>

@BubbleCal BubbleCal 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.

pending CI

@jiaqizho

jiaqizho commented Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

pending CI

hi @BubbleCal , no sure why this python CI failed.

[Python / Python Linux 3.10 x86_64 (pull_request)](https://github.com/lance-format/lance/actions/runs/29479473607/job/87791052270?pr=7820)Failing after 14m

could u retrigger it? thanks

@BubbleCal

Copy link
Copy Markdown
Contributor

pending CI

hi @BubbleCal , no sure why this python CI failed.

[Python / Python Linux 3.10 x86_64 (pull_request)](https://github.com/lance-format/lance/actions/runs/29479473607/job/87791052270?pr=7820)Failing after 14m

could u retrigger it? thanks

Seems flaky, just reran it

@codecov

codecov Bot commented Jul 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@BubbleCal
BubbleCal merged commit f23b8d6 into lance-format:main Jul 17, 2026
45 of 47 checks passed
wjones127 pushed a commit that referenced this pull request Jul 21, 2026
Make the full and indexed file metadata lookup methods public so callers
can reuse the fragment's dataset-level metadata cache instead of loading
metadata independently through FileReader.

I ran into this while trying to inspect the file footer of a fragment
that had already been opened. The obvious option was to call
FileReader::read_all_metadata, but that API reads directly from storage
and does not reuse the dataset’s file metadata cache. This can result in
another metadata I/O even though the fragment has already loaded the
same information.

FileFragment already has cache-aware helpers for loading the full
metadata or metadata index. This change makes get_file_metadata and
get_file_metadata_index public so other fragment-level operations can
reuse the existing cached metadata instead of reading it again through
FileReader.

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **New Features**
* Exposed file metadata and metadata index access for broader
integration and tooling.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

(cherry picked from commit f23b8d6)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants