Use spanvalue WriteRowIterator for experimental CSV

[apstndb]

Summary

• Replace the hand-rolled RowIterator loop in writeCsvFromRowIter with svwriter.WriteRowIterator.
• Keep --redact-rows as skipRowIter followed by WriteRowIterator (header-only when metadata has columns).
• WriteRowIterator owns Stop, PrepareRowType on first Next, and Flush via hooks.

Why

Aligns experimental CSV with spanvalue v0.5.0 documented RowIterator path (zero-row SELECT, redact) and drops ~25 lines of duplicate lifecycle code.

Test plan

• go test ./...

Made with Cursor

[gemini-code-assist]

Code Review

This pull request simplifies writeCsvFromRowIter by replacing manual row iteration with svwriter.WriteRowIterator. However, removing defer rowIter.Stop() introduces a potential resource leak if skipRowIter returns an error early when redactRows is true.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[apstndb]

Nice cleanup. I traced both paths against the spanner v1.84.1 and spanvalue v0.5.0 sources and the refactor is behavior-preserving:

• non-redact: WriteRowIterator (runRowIterator, writer/row_iterator.go:252-285) runs the exact same logic as the old hand-rolled loop — PrepareMetadata on the first Next (even when it's iterator.Done), WriteRow per row, Flush in Finish. Same header/row output.
• redact: skipRowIter -> RowIterator.Do drains to Done and always calls Stop (spanner/read.go:269), leaving r.err == iterator.Done. The following WriteRowIterator's first Next short-circuits on that r.err (read.go:162) and returns Done, so it emits header-only and Flushes — same output as the old prepareCsvRowType + Flush.
• No Stop leak: Do always Stops (covers the skipRowIter error path too) and WriteRowIterator always Stops, so dropping defer rowIter.Stop() is safe. The only theoretical gap is the unreachable NewCSVWriter error before the iterator is ever driven — negligible.
• go build ./... and go test -run TestExperimentalCsv . pass.

Two non-blocking points inline, plus one coverage note:

Test coverage: writeCsvFromRowIter (the production CSV streaming path, both normal and redact) is not exercised by any unit or integration test — csv_test.go covers writeCsvFromResultSet and a separate hand-rolled writeCsvFromPreparedRows, and integration_test.go has no experimental_csv case. This refactor touches the one CSV path that has no regression test. Worth an emulator-backed (spanemuboost) test that runs writeCsvFromRowIter for normal, zero-row, and --redact-rows, especially since the redact header-only behavior is the subtle part.

[apstndb]

Re-reviewed at eecae90. The new csvRedactRowIteratorWriter approach cleanly resolves my earlier concern about re-driving a drained+stopped iterator — both paths now go through a single WriteRowIterator call that owns Stop, with no double-Stop and no reliance on Next-after-Done semantics. Nice.

Verified the new design against the spanvalue v0.5.0 source:

• RowIteratorWriter embeds FlushWriter (= Writer + Flusher), so it includes WriteRow, Flush, and PrepareRowType. RowIteratorHooksFromWriter captures w.WriteRow from the interface value, so dynamic dispatch picks csvRedactRowIteratorWriter's explicit nop WriteRow (which shadows the promoted *DelimitedWriter.WriteRow). Redact output is therefore header-only: PrepareMetadata -> header, nop WriteRow discards each row, Finish -> Flush.
• The embedded field is a pointer (*svwriter.DelimitedWriter), so the csvRedactRowIteratorWriter value satisfies RowIteratorWriter (promoted pointer methods are in the value's method set).
• skipRowIter is no longer used by the CSV path but is still used at main.go:577 (JSON/jq redact) and in integration_test.go, so it is not dead code.
• go build ./..., go vet ./..., and go test -run TestExperimentalCsv . all pass.

Note: redact still streams every row from the server and discards it (same as the old skipRowIter/Do), so there is no behavior change there — redact suppresses output, not the scan.

One remaining ask inline: this redact path is still untested, and its correctness now hinges on Go method-shadowing through an interface.