feat: Introduce docs for paimon-rust#157
Conversation
QuakeWang
left a comment
QuakeWang
left a comment
There was a problem hiding this comment.
Hi @JingsongLi, I have reviewed the documents and left some minor comments.
| [dependencies] | ||
| paimon = "0.0.0" | ||
| paimon-datafusion = "0.0.0" | ||
| datafusion = "46" |
There was a problem hiding this comment.
| datafusion = "46" | |
| datafusion = "52" |
The DataFusion version here is out of date. The integration crate currently depends on 52.3.0 — using "46" will cause API incompatibility at compile time. Please update to datafusion = "52".
|
|
||
| # Documentation | ||
|
|
||
| This directory contains the source files for the Apache Paimon Rust documentation site, built with [MkDocs](https://www.mkdocs.org/) and the [Material for MkDocs](https://squidfun.github.io/mkdocs-material/) theme. |
There was a problem hiding this comment.
| This directory contains the source files for the Apache Paimon Rust documentation site, built with [MkDocs](https://www.mkdocs.org/) and the [Material for MkDocs](https://squidfun.github.io/mkdocs-material/) theme. | |
| This directory contains the source files for the Apache Paimon Rust documentation site, built with [MkDocs](https://www.mkdocs.org/) and the [Material for MkDocs](https://squidfunk.github.io/mkdocs-material/) theme. |
| let read_builder = table.new_read_builder(); | ||
|
|
||
| // Step 1: Scan — produces a Plan containing DataSplits | ||
| let scan = read_builder.new_scan(); |
There was a problem hiding this comment.
This example will not compile. new_scan() borrows read_builder immutably and the returned TableScan<'a> keeps that borrow alive as long as scan is in scope. Calling read_builder.new_read() on line 119 while scan still holds the borrow will be rejected by the borrow checker. Split the two phases into separate scopes:
// Scan phase — borrow ends when this block exits
let plan = {
let scan = read_builder.new_scan();
scan.plan().await?
};
// Read phase
let reader = read_builder.new_read()?;
let mut stream = reader.to_arrow(plan.splits())?;| cargo fmt | ||
|
|
||
| # Lint | ||
| cargo clippy |
There was a problem hiding this comment.
The project CI runs cargo clippy --all-targets --workspace -- -D warnings. Running cargo clippy locally without -D warnings may pass while CI fails, creating a confusing contributor experience. Please align this with the actual CI command.
luoyuxia
left a comment
luoyuxia
left a comment
There was a problem hiding this comment.
@JingsongLi Thanks for the great pr. The website style/theme looks well. Just left minor comments.
|
|
||
| site_name: Apache Paimon Rust | ||
| site_description: The Rust implementation of Apache Paimon | ||
| site_url: https://paimon.apache.org/paimon-rust/ |
There was a problem hiding this comment.
seem it's not available now.
Another question is do we need a separate website to hold the docs just like cpp client https://alibaba.github.io/paimon-cpp/index.html
There was a problem hiding this comment.
I will add this in paimon-web project.
| - REST Catalog client | ||
| - Apache DataFusion integration | ||
| - Partitioned table support | ||
| - C FFI bindings |
| Planned features: | ||
|
|
||
| - Paimon table format reader | ||
| - Local filesystem and S3 storage backends |
| Key features: | ||
|
|
||
| - Native Rust reader for Paimon table format | ||
| - Support for local filesystem and S3 storage backends |
There was a problem hiding this comment.
dito: we also support oss
|
|
||
| while let Some(batch) = stream.next().await { | ||
| let batch = batch?; | ||
| println!("Got batch with {} rows", batch.num_rows()); |
There was a problem hiding this comment.
nit: may be print the record batch
println!("RecordBatch: {batch:#?}");
|
|
||
| The core crate implements the Paimon table format, including: | ||
|
|
||
| - **Catalog** — REST Catalog client for discovering and managing databases and tables |
There was a problem hiding this comment.
| - **Catalog** — REST Catalog client for discovering and managing databases and tables | |
| - **Catalog** — Catalog client for discovering and managing databases and tables |
Since not only rest catalog, but also filesystem catalog
| protected_branches: | ||
| main: | ||
| required_pull_request_reviews: | ||
| required_approving_review_count: 0 |
There was a problem hiding this comment.
| required_approving_review_count: 0 | |
| required_approving_review_count: 1 |
I think this value is at least 1. Please verify this is the intended value before merging.
There was a problem hiding this comment.
Just test review override.
This setting makes merging cumbersome, as two committers must participate in order to merge the code.
3 participants
Purpose
Introduce docs.
Brief change log
Tests
API and Format
Documentation