Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

61 changes: 26 additions & 35 deletions rust/lance-table/src/io/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,25 @@ use crate::format::{DataStorageFormat, IndexMetadata, MAGIC, Manifest, Transacti

use super::commit::ManifestLocation;

/// Read Manifest on URI.
/// Read the raw Manifest protobuf from a URI.
///
/// This only reads manifest files. It does not read data files.
/// This only reads manifest files. It does not read data files or translate the
/// protobuf into the semantic [`Manifest`] type.
Comment on lines +30 to +33

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.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Document both public manifest readers with synchronized examples.

  • rust/lance-table/src/io/manifest.rs#L30-L33: add a compiling example for read_manifest_proto and link its protobuf/semantic output types.
  • rust/lance-table/src/io/manifest.rs#L120-L122: add a compiling example for read_manifest and link relevant types or methods.

As per coding guidelines, “Document all public APIs with examples and links to relevant structs and methods; keep examples synchronized with actual signatures.”

📍 Affects 1 file
  • rust/lance-table/src/io/manifest.rs#L30-L33 (this comment)
  • rust/lance-table/src/io/manifest.rs#L120-L122
🤖 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-table/src/io/manifest.rs` around lines 30 - 33, Add synchronized,
compiling Rustdoc examples to the public readers read_manifest_proto
(rust/lance-table/src/io/manifest.rs:30-33) and read_manifest
(rust/lance-table/src/io/manifest.rs:120-122), using their current signatures
and documenting links to the relevant protobuf and semantic Manifest types or
methods. Ensure both examples remain valid if the APIs change.

Source: Coding guidelines

#[instrument(level = "debug", skip(object_store))]
pub async fn read_manifest(
pub async fn read_manifest_proto(
object_store: &ObjectStore,
path: &Path,
known_size: Option<u64>,
) -> Result<Manifest> {
) -> Result<pb::Manifest> {
let buf = read_manifest_bytes(object_store, path, known_size).await?;
Ok(pb::Manifest::decode(buf)?)
Comment on lines +35 to +41

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.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Add direct coverage for the new raw-protobuf API.

test_roundtrip_manifest only verifies read_manifest; add tests that assert read_manifest_proto returns the expected protobuf and that a stale known_size retries successfully.

As per coding guidelines, “Every bugfix and 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-table/src/io/manifest.rs` around lines 35 - 41, Add direct tests
for read_manifest_proto covering normal round-trip decoding to the expected
pb::Manifest and a stale known_size value that successfully retries and returns
the correct protobuf. Extend or add focused manifest tests alongside
test_roundtrip_manifest, reusing existing object-store and manifest setup.

Source: Coding guidelines

}

async fn read_manifest_bytes(
object_store: &ObjectStore,
path: &Path,
known_size: Option<u64>,
) -> Result<Bytes> {
let file_size = if let Some(known_size) = known_size {
known_size
} else {
Expand All @@ -52,7 +62,7 @@ pub async fn read_manifest(
// In case of corruption, the known_size might be wrong. We can retry without
// the size to be more robust.
if (buf.len() < 16 || !buf.ends_with(MAGIC)) && known_size.is_some() {
return Box::pin(read_manifest(object_store, path, None)).await;
return Box::pin(read_manifest_bytes(object_store, path, None)).await;
}

if buf.len() < 16 {
Expand Down Expand Up @@ -104,39 +114,20 @@ pub async fn read_manifest(
)));
}

let proto = pb::Manifest::decode(buf)?;
Manifest::try_from(proto)
Ok(buf)
}

#[instrument(level = "debug", skip(object_store, manifest))]
pub async fn read_manifest_indexes(
/// Read Manifest on URI.
///
/// This only reads manifest files. It does not read data files.
#[instrument(level = "debug", skip(object_store))]
pub async fn read_manifest(
object_store: &ObjectStore,
location: &ManifestLocation,
manifest: &Manifest,
) -> Result<Vec<IndexMetadata>> {
if let Some(pos) = manifest.index_section.as_ref() {
let result = read_index_section(object_store, &location.path, location.size, *pos).await;
// A stale cached size makes the index offset fall outside the sized view,
// so the read fails as "file size is too small". Retry once with the true
// size; surface any other error unchanged.
let section = match result {
Err(e)
if location.size.is_some() && e.to_string().contains("file size is too small") =>
{
read_index_section(object_store, &location.path, None, *pos).await?
}
other => other?,
};

let indices = section
.indices
.into_iter()
.map(IndexMetadata::try_from)
.collect::<Result<Vec<_>>>()?;
Ok(indices)
} else {
Ok(vec![])
}
path: &Path,
known_size: Option<u64>,
) -> Result<Manifest> {
let proto = read_manifest_proto(object_store, path, known_size).await?;
Manifest::try_from(proto)
}

/// Read the index section message at `pos`, opening the manifest with a known
Expand Down
3 changes: 3 additions & 0 deletions rust/lance-tools/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,13 @@ name = "lance-tools"
path = "src/main.rs"

[dependencies]
chrono.workspace = true
clap = { workspace = true, features = ["derive"] }
lance-core.workspace = true
lance-file.workspace = true
lance-io.workspace = true
lance-table.workspace = true
object_store.workspace = true
prost.workspace = true
tokio.workspace = true
url.workspace = true
14 changes: 14 additions & 0 deletions rust/lance-tools/README.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,17 @@
# lance-tools

`lance-tools` is a command-line tool for interacting with Lance files and tables.

## Commands

Display Lance file footer metadata:

```bash
lance-tools file meta --source path/to/file.lance
```

Display a Lance table manifest:

```bash
lance-tools table manifest --source path/to/table/_versions/1.manifest
```
26 changes: 26 additions & 0 deletions rust/lance-tools/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ pub struct LanceToolsArgs {
pub enum LanceToolsCommand {
/// Commands for interacting with Lance files.
File(LanceFileArgs),
/// Commands for interacting with Lance tables.
Table(LanceTableArgs),
}

#[derive(Parser, Debug)]
Expand All @@ -34,19 +36,43 @@ pub enum LanceFileCommand {
Meta(LanceFileMetaArgs),
}

#[derive(Parser, Debug)]
pub struct LanceTableArgs {
#[command(subcommand)]
command: LanceTableCommand,
}

#[derive(Subcommand, Debug)]
pub enum LanceTableCommand {
/// Display Lance table manifest contents.
Manifest(LanceTableManifestArgs),
}

#[derive(Args, Debug)]
pub struct LanceFileMetaArgs {
// The Lance file to examine.
#[arg(short = 's', long, value_name = "source")]
pub(crate) source: String,
}

#[derive(Args, Debug)]
pub struct LanceTableManifestArgs {
/// The Lance manifest file to examine.
#[arg(short = 's', long, value_name = "source")]
pub(crate) source: String,
}

impl LanceToolsArgs {
pub async fn run(&self, writer: impl std::io::Write) -> Result<()> {
match &self.command {
LanceToolsCommand::File(args) => match &args.command {
LanceFileCommand::Meta(args) => crate::meta::show_file_meta(writer, args).await,
},
LanceToolsCommand::Table(args) => match &args.command {
LanceTableCommand::Manifest(args) => {
crate::manifest::show_table_manifest(writer, args).await
}
},
}
}
}
1 change: 1 addition & 0 deletions rust/lance-tools/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,6 @@
// SPDX-FileCopyrightText: Copyright The Lance Authors

pub mod cli;
pub mod manifest;

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.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Files:"
git ls-files | rg '(^|/)lib\.rs$|(^|/)cli\.rs$|(^|/)manifest\.rs$|Cargo\.toml$'

echo
echo "lib.rs:"
sed -n '1,80p' rust/lance-tools/src/lib.rs

echo
echo "manifest.rs outline:"
ast-grep outline rust/lance-tools/src/manifest.rs --view compact || true

echo
echo "manifest.rs relevant entries:"
rg -n "^(pub )?|show_table_manifest|TableManifest|struct |enum " rust/lance-tools/src/manifest.rs

echo
echo "cli.rs manifest usages:"
rg -n "crate::manifest|manifest::|show_table_manifest|lance_tools::manifest" rust/lance-tools/src/cli.rs

echo
echo "workspace/crate config relevant:"
sed -n '1,160p' rust/lance-tools/Cargo.toml

Repository: lance-format/lance

Length of output: 50374


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "lance-tools package metadata:"
python3 - <<'PY'
import configparser, re, json
p='rust/lance-tools/Cargo.toml'
text=open(p).read()
print(text)
PY

echo
echo "manifest struct visibility and internal call path:"
rg -n -C 3 '^(pub )?(struct LanceToolManifest|enum OptionalSection|struct IndexView|struct TransactionView|struct TransactionSource|struct ManifestSource|async fn open|impl LanceToolManifest|pub\(crate\) async fn show_table_manifest)' rust/lance-tools/src/manifest.rs
rg -n -C 2 'LanceTableManifestArgs|show_table_manifest|crate::manifest::manifest_path|manifest::show_table_manifest' rust/lance-tools/src/cli.rs rust/lance-tools/src/*.rs

echo
echo "all external references to lance_tools::manifest / crate::manifest outside lance-tools:"
rg -n 'lance_tools::manifest|lance-tools::manifest|lance_tools::util|crate::manifest|manifest::show_table_manifest' --glob '!rust/lance-tools/**' .

echo
echo "all references to public types in manifest.rs (excluding tests/internal helpers if apparent):"
rg -n 'LanceToolManifest|OptionalSection|IndexView|TransactionView|TransactionSource|ManifestSource' rust/lance-tools/src/manifest.rs

Repository: lance-format/lance

Length of output: 5693


Keep the manifest module crate-private.

manifest is only reached from lance-tools CLI (crate::manifest::show_table_manifest), and the CLI entrypoint itself is pub(crate). Use pub(crate) mod manifest; unless lance_tools::manifest is intended as public API.

🤖 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-tools/src/lib.rs` at line 5, Change the manifest module
declaration to crate visibility by updating pub mod manifest to pub(crate) mod
manifest; in the crate root. Preserve internal access from the CLI entrypoint
through crate::manifest::show_table_manifest without exposing
lance_tools::manifest publicly.

Source: Coding guidelines

pub mod meta;
pub mod util;
Loading
Loading