Copilot commented on code in PR #271:
URL: https://github.com/apache/fluss-rust/pull/271#discussion_r2777477607
##########
README.md:
##########
@@ -101,7 +101,8 @@ pub async fn main() -> Result<()> {
let table = conn.get_table(&table_path).await;
let append_writer = table.new_append().create_writer();
let batch = record_batch!(("c1", Int32, [1, 2, 3, 4, 5, 6]), ("c2", Utf8,
["a1", "a2", "a3", "a4", "a5", "a6"])).unwrap();
- append_writer.append(batch).await?;
+ append_writer.append(batch)?;
Review Comment:
This example appends an Arrow `RecordBatch`, but the Rust API uses
`append_arrow_batch(batch)` for batches (while `append(&row)` is for rows).
Consider updating the README snippet to call the correct method, and (for
fire-and-forget) bind/drop the returned `WriteResultFuture` to avoid
`unused_must_use` warnings.
```suggestion
let _write_result = append_writer.append_arrow_batch(batch)?;
```
##########
crates/fluss/tests/integration/log_table.rs:
##########
@@ -99,14 +99,12 @@ mod table_test {
record_batch!(("c1", Int32, [1, 2, 3]), ("c2", Utf8, ["a1", "a2",
"a3"])).unwrap();
append_writer
.append_arrow_batch(batch1)
- .await
.expect("Failed to append batch");
Review Comment:
`append_arrow_batch(...)` returns a `WriteResultFuture` (a `Future`), so
ignoring the returned handle will generally trigger `unused_must_use` warnings.
If this is intended to be fire-and-forget before `flush()`, bind or explicitly
drop the handle to document intent and keep the tests warning-free.
##########
crates/examples/src/example_table.rs:
##########
@@ -64,12 +64,12 @@ pub async fn main() -> Result<()> {
let table = conn.get_table(&table_path).await?;
let append_writer = table.new_append()?.create_writer()?;
// Fire-and-forget: queue writes then flush
- append_writer.append(&row).await?;
+ append_writer.append(&row)?;
let mut row = GenericRow::new(3);
Review Comment:
`append_writer.append(&row)?;` returns a `WriteResultFuture` (a `Future`),
so ignoring it will usually trigger `unused_must_use` warnings. If this is
intended to be fire-and-forget, bind or explicitly drop the returned handle
(e.g., `let _ = ...` / `drop(...)`).
##########
crates/fluss/tests/integration/kv_table.rs:
##########
@@ -101,10 +101,7 @@ mod kv_table_test {
row.set_field(0, *id);
row.set_field(1, *name);
row.set_field(2, *age);
- upsert_writer
- .upsert(&row)
- .await
- .expect("Failed to upsert row");
+ upsert_writer.upsert(&row).expect("Failed to upsert row");
Review Comment:
`upsert_writer.upsert(&row).expect(...)` returns a `WriteResultFuture` (a
`Future`), so dropping it in statement position will typically emit an
`unused_must_use` warning. If the intent is fire-and-forget before `flush()`,
bind or explicitly drop the handle (e.g., `let _ = ...` / `drop(...)`).
```suggestion
let _ = upsert_writer.upsert(&row).expect("Failed to upsert
row");
```
##########
crates/examples/src/example_partitioned_kv_table.rs:
##########
@@ -74,7 +74,7 @@ pub async fn main() -> Result<()> {
row.set_field(1, region);
row.set_field(2, zone);
row.set_field(3, score);
- upsert_writer.upsert(&row).await?;
+ upsert_writer.upsert(&row)?;
Review Comment:
`upsert_writer.upsert(&row)?;` returns a `WriteResultFuture` (a `Future`),
so ignoring it can trigger `unused_must_use` warnings. If this is intentional
fire-and-forget behavior, bind or explicitly drop the handle to make the intent
clear and avoid warnings.
```suggestion
upsert_writer.upsert(&row).await?;
```
##########
crates/fluss/src/client/write/mod.rs:
##########
@@ -205,8 +205,8 @@ impl ResultHandle {
/// A future that represents a pending write operation.
///
/// This type implements [`Future`], allowing users to either:
-/// 1. Await immediately to block on acknowledgment:
`writer.upsert(&row).await?.await?`
-/// 2. Fire-and-forget with later flush: `writer.upsert(&row).await?;
writer.flush().await?`
+/// 1. Await immediately to block on acknowledgment:
`writer.upsert(&row)?.await?`
+/// 2. Fire-and-forget with later flush: `writer.upsert(&row)?;
writer.flush().await?`
Review Comment:
The fire-and-forget example (`writer.upsert(&row)?; writer.flush().await?`)
will trigger Rust’s `unused_must_use` lint because `WriteResultFuture`
implements `Future` (and `Future` is `#[must_use]`). Prefer explicitly
binding/dropping the handle (e.g., `let _ = writer.upsert(&row)?;` or
`drop(writer.upsert(&row)?);`) so examples compile cleanly and the intent is
clear.
```suggestion
/// 2. Fire-and-forget with later flush: `let _ = writer.upsert(&row)?;
writer.flush().await?`
```
##########
crates/examples/src/example_kv_table.rs:
##########
@@ -62,7 +62,7 @@ pub async fn main() -> Result<()> {
row.set_field(0, id);
row.set_field(1, name);
row.set_field(2, age);
- upsert_writer.upsert(&row).await?;
+ upsert_writer.upsert(&row)?;
Review Comment:
`upsert_writer.upsert(&row)?;` returns a `WriteResultFuture` (a `Future`),
so ignoring it can trigger `unused_must_use` warnings. If this is intentional
fire-and-forget behavior before `flush()`, bind or explicitly drop the handle.
```suggestion
let _write_result = upsert_writer.upsert(&row)?;
```
--
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]
