rickard-lundin commented on PR #2998:
URL: https://github.com/apache/iceberg-rust/pull/2998#issuecomment-5379395206
@blackmwk you are right. I tried it, and the speedup is reached through your
suggestion — `fast_append().with_check_duplicate(false)` on `main` gives the
same thing. I didn't see that possibility when I opened this, sorry for the
noise.
It is a bit "very much coding" to get there though (see below): to get the
updates out without a real catalog round-trip I had to implement `Catalog` with
13 of 15 methods as `unimplemented!()`, and rebuild the `Table` through
`Table::builder()` since `with_metadata` is `pub(crate)`. Works fine, feels
wrong.
So I'm reframing the PR: dropping `stage_fast_append`, and instead adding
one small, action-agnostic `Transaction::stage(&self) -> Result<TableCommit>` —
the first half of `commit` (same loop, `do_commit` now shares it), it just
returns the `TableCommit` instead of sending it. `Catalog::update_table`
already takes a `TableCommit`, so it round-trips as-is. Title/description
rewritten, speed claims gone, tests and `public-api.txt` updated.
If you'd rather not have this at all, say so and I'll close it and keep the
shim downstream. No hard feelings.
<details><summary>the workaround on main</summary>
```rust
#[derive(Debug)]
struct StagingCatalog { table: Table, captured:
Mutex<Option<(Vec<TableUpdate>, Vec<TableRequirement>)>> }
#[async_trait]
impl Catalog for StagingCatalog {
async fn load_table(&self, _: &TableIdent) -> Result<Table> {
Ok(self.table.clone()) }
async fn update_table(&self, mut commit: TableCommit) -> Result<Table> {
let updates = commit.take_updates();
let requirements = commit.take_requirements();
let mut b = self.table.metadata().clone().into_builder(None);
for u in &updates { b = u.clone().apply(b)?; }
let t = Table::builder()
.identifier(self.table.identifier().clone())
.file_io(self.table.file_io().clone())
.metadata(b.build()?.metadata)
.runtime(Runtime::current())
.build()?;
*self.captured.lock().unwrap() = Some((updates, requirements));
Ok(t)
}
// ... 13 more methods, all unimplemented!()
}
let shim = StagingCatalog { table: table.clone(), captured: Mutex::new(None)
};
let tx = Transaction::new(&table);
let tx =
tx.fast_append().with_check_duplicate(false).add_data_files(files).apply(tx)?;
tx.commit(&shim).await?;
let (updates, requirements) = shim.captured.lock().unwrap().take().unwrap();
```
Measured on `main` through this, staging a 1-file append onto a table with N
live files (local fs): 1004 files 69.5 ms → 3.5 ms, 4004 files 282 ms → 16.6 ms.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
