Add a safe reusable row API for appenders

[skuirrels]

Summary

The first version of this PR cached the row returned by CreateRow(). Further
testing showed that code retaining that reference could accidentally write to a
later row.

I've changed the approach so CreateRow() keeps its existing behaviour and
continues to return an independent row. Reuse is now available through:

• An internal CreateReusableRow() method for code that owns the complete row
  lifecycle.
• Public AppendRow() overloads that limit the reusable row to a callback.
• DuckDBMappedAppender, which now uses AppendRow() internally.

appender.AppendRow(row =>
{
    row.AppendValue((int?)42);
    row.AppendValue("answer");
});

appender.AppendRow(record, static (row, value) =>
{
    row.AppendValue(value.Id);
    row.AppendValue(value.Name);
});

AppendRow() calls EndRow() automatically. If the callback or EndRow()
fails, all previously completed rows are flushed, the failed row is excluded,
and the appender is closed and faulted so it cannot be reused. An explicit
caller-owned transaction can still roll back the completed rows.

Benchmark

1,000,000 rows with four columns, .NET 10:

Path	Mean	Allocated	Gen0
CreateRow()	36.52 ms	62.59 MB	7
AppendRow()	30.76 ms	96 KB	0

In this run, AppendRow() was about 16% faster and reduced managed allocations
by 99.85%.

The mapped appender shows the same allocation improvement, dropping from roughly
61 MB to 93 KB per million rows. Its timings moved around between short local
runs, so I wouldn't draw a firm CPU-performance conclusion from those results.

Tests

Tests cover independent CreateRow() instances, both AppendRow() overloads,
callback and EndRow() failures, vector boundaries, caller-owned transaction
rollback, stale row references, finalization failures, repeated disposal, null
callbacks, reentrant use and list values.

The full local suite passes: 6,908 tests.

Benchmark project

The BenchmarkDotNet project targets .NET 10 and is signed like the other
projects. It uses the in-process toolchain because the repository build renames
the output assembly, which prevents the default toolchain from finding the
project file.

[codecov]

Codecov Report

❌ Patch coverage is 96.20253% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.92%. Comparing base (6f5a68d) to head (68195de).
⚠️ Report is 2 commits behind head on develop.

Files with missing lines	Patch %	Lines
DuckDB.NET.Data/Mapping/DuckDBAppenderMap.cs	40.00%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #336      +/-   ##
===========================================
+ Coverage    87.58%   87.92%   +0.33%     
===========================================
  Files           77       77              
  Lines         3134     3205      +71     
  Branches       463      470       +7     
===========================================
+ Hits          2745     2818      +73     
  Misses         275      275              
+ Partials       114      112       -2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[skuirrels]

Just flagging that the macOS red X isn't from this change — it's the Coveralls upload step, which dies trying to brew install its reporter ("Refusing to load formula ... from untrusted tap"). Homebrew changed how it trusts taps and the coveralls action hasn't caught up, so it's failing on macOS for any PR right now, not just this one. The build and tests themselves pass fine on macOS, and Ubuntu/Windows are both green.

Happy to send a small CI fix for it separately if that's helpful.

[Giorgi]

I remember writing the exact same code but I don't remember why I didn't ship it 🤔

[skuirrels]

Maybe you forgot 😊. I'll do a little more research to look for potential side effects. I use this in a large application and can also run it with my fork and put it through some tests. Will revert tomorrow night.

[skuirrels]

I tested this further and found that reusing the object returned by CreateRow() can cause a retained row reference to write into a later row.
I’ve revised the approach:

1. CreateRow() keeps its existing behaviour and returns an independent row.
2. Add internal CreateReusableRow() for controlled paths such as DuckDBMappedAppender.
3. Add AppendRow() as a scoped public API so bulk integrations can benefit from reuse without receiving the reusable row as a return value.

This preserves compatibility while still enabling the allocation reduction for mapped, bulk-insert and upsert paths. I’m validating the updated implementation and benchmarks locally before updating the PR tomorrow.

[skuirrels]

Ok, so it seems GitHub automatically updated the PR when pushing to my fork, despite no explicit PR action. I'll refine this and confirm back later, so ignore for now.

[Giorgi]

@skuirrels BTW, feel free to join the dotnet channel on https://discord.duckdb.org/ if you want to discuss your approach before implementing it.

[skuirrels]

Updated following the additional testing. CreateRow() keeps its independent-row behaviour, while reusable rows are now provided through scoped AppendRow() callbacks and controlled internal paths. The lifetime and failure tests, benchmark project and benchmark results have also been updated.

[skuirrels]

@skuirrels BTW, feel free to join the dotnet channel on https://discord.duckdb.org/ if you want to discuss your approach before implementing it.

Ok, will send you a message there now.

[Giorgi]

I can mark the CreateRow method as Obsolete and add a new method AppendRow that has the same signature as CreateRow and also mention in XML comments that its result should not be retained.

[skuirrels]

My only concern is that this makes stale-reference safety documentation-based and loses the callback overload’s automatic EndRow() and failure cleanup. Would you prefer the parameterless method to replace the callback overloads, or keep the callback overload as the safer scoped option?

[Giorgi]

I think rolling back the batch is not a good idea. First, the batch size depends on the DuckDB version, as duckdb_vector_size may change in the future. Secondly, depending on which row faults, the number of rows rolled back varies, making it hard to tell callers exactly which rows are discarded.

I think we should just put the appender in faulted state and not allow any other operation on it, including committing the already written rows to disc. What do you think?

[Giorgi]

Alternatively, when writeRow action throws, we could write and commit all the rows before the current one instead of discarding the chunk, and put the appender in faulted state.

[skuirrels]

I agree. I prefer the second option: keep all successfully completed rows, discard the failing row, then fault the appender so it can’t be reused. We should call this “flushing” rather than “committing,” since the caller may be using a transaction.
I’ll update the implementation and add tests around the vector boundaries.

[skuirrels]

Updated this based on the feedback.

If the callback or automatic EndRow() fails, all previously completed rows are flushed, the failed row is excluded, and the appender is closed and faulted so it cannot be reused. With an explicit transaction, the caller can still roll everything back.

I also added coverage for vector boundaries, transaction rollback, finalization failures, repeated disposal, and closed/faulted operations.

Validation: 37/37 managed appender tests and 6,908/6,908 full tests passed.