fix(parquet): add WriteBatchSpacedWithError to surface spaced-write failures

[zeroshade]

Rationale for this change

Closes #826.

Across all 8 generated *ColumnChunkWriter types in parquet/file/column_writer_types.gen.go, WriteBatch and WriteBatchSpaced are an asymmetric pair:

• WriteBatch returns (valueOffset int64, err error) and opens with a defer recover() that converts any panic into a returned error via utils.FormatRecoveredError.
• WriteBatchSpaced returns nothing, has no defer recover(), and on the doBatches (non-byte-array) path silently discards the error returned by commitWriteAndCheckPageLimit.

So a write failure during a spaced write is either silently discarded (numeric/boolean/fixed paths) or escapes as a panic out of the calling goroutine (the byte-array branch's panic(err), plus other internal panics such as dictionary-fallback bookkeeping) — because, unlike WriteBatch, there is no recover() to turn it into an error.

This is the same class of "panic escapes / error lost on the public API" bug as #820 (fixed in #824), on a different code path. It is reachable through pqarrow — (*pqarrow.FileWriter) drives WriteBatchSpaced for nullable columns — so for consumers writing nullable columns to a network-attached sink (GCS, S3 multipart, HDFS, etc.), a transient downstream failure on the spaced path crashes the worker process or silently drops the error instead of returning it.

The Boolean-only WriteBitmapBatchSpaced has the same void-signature / no-recover shape and is fixed here too.

What changes are included in this PR?

1. New method WriteBatchSpacedWithError(values, defLevels, repLevels, validBits, validBitsOffset) (int64, error) generated for all 8 column-writer types from column_writer_types.gen.go.tmpl. It mirrors WriteBatch:

   • opens with defer func() { if r := recover(); r != nil { err = utils.FormatRecoveredError(...) } }(), and
   • checks the error from commitWriteAndCheckPageLimit on every branch (the doBatches branch previously ignored it), panic-ing so the deferred recover surfaces it as a returned error.

2. New method WriteBitmapBatchSpacedWithError(...) (int64, error) on *BooleanColumnChunkWriter, applying the same treatment to the bitmap spaced-write path.

3. WriteBatchSpaced and WriteBitmapBatchSpaced are left byte-for-byte unchanged — same signatures, same behavior. This keeps the change non-breaking and purely additive, exactly the approach fix(parquet): return error instead of panicking on first-write failure #824 took with NewParquetWriter / NewParquetWriterWithError. Existing callers are unaffected; callers that want error handling opt into the …WithError variants.

4. pqarrow's dense-array encoder (writeDenseArrow) now calls WriteBatchSpacedWithError at all 12 spaced-write sites and propagates the error through its existing (err error) return, so nullable-column writes report sink failures as errors instead of crashing the goroutine or losing the error. (WriteBitmapBatchSpaced has no internal callers, so nothing to rewire there.)

The diff to the generated file and the template is additions-only (no existing generated method bodies change).

Are these changes tested?

Yes. Two new tests in parquet/file/column_writer_test.go use the existing mockpagewriter to fail WriteDataPage, with a tiny DataPageSize and dictionary disabled to force a flush mid-call:

• TestWriteBatchSpacedWithErrorPropagatesWriteFailure
• TestWriteBitmapBatchSpacedWithErrorPropagatesWriteFailure

Each asserts that the …WithError variant returns the wrapped failure (errors.Is(err, failureErr)) rather than panicking or discarding it, and that the legacy method does not panic out of the call (its original behavior is preserved).

The full parquet/file/... and parquet/pqarrow/... suites pass (PARQUET_TEST_DATA pointed at parquet-testing/data), and go vet is clean.

Are there any user-facing changes?

No breaking changes.

• WriteBatchSpaced(...) and WriteBitmapBatchSpaced(...) — signatures and behavior unchanged.
• WriteBatchSpacedWithError(...) (int64, error) (all 8 types) and WriteBitmapBatchSpacedWithError(...) (int64, error) (Boolean) — new exported methods. Adding exported methods is non-breaking in Go.
• pqarrow public signatures are unchanged; the dense-array nullable write path now returns transient sink failures as an error (callers already checking err get strictly better behavior).

[lidavidm]

WriteBatchSpaced is internal, right? Why keep it at this point if it's not a public API?

[zeroshade]

Both are technically part of the public API and I'm trying to avoid making breaking changes, it isn't inside of an internal package and it starts with a capital letter, so it's publicly accessible.

[lidavidm]

(1) Could we deprecate WriteBatchSpaced?
(2) Could WriteBatchSpaced be written in terms of WriteBatchSpacedWithError instead of duplicating the code entirely? (e.g. defer to WriteBatchSpacedWithError and panic if it returns an error)

[zeroshade]

Done in 975d052.

1. Deprecated — both WriteBatchSpaced (all 8 column-writer types) and WriteBitmapBatchSpaced now carry a // Deprecated: notice pointing at their …WithError counterparts.
2. No more duplication — each now just defers to the …WithError variant and panics if it returns an error, so the duplicated bodies are gone (−265 lines in the generated file; template + regen).

One behavior note worth your eyes: the legacy methods now fail loudly on a write error instead of silently discarding it on the numeric/boolean/fixed paths (the byte-array path already panicked). Because the delegation routes through …WithError's recover, the panic payload is now the FormatRecoveredError("unknown error type", …) wrapper rather than the raw error — errors.Is(…, original) still resolves through the %w chain, but the exact value differs for the byte-array/FLBA paths that previously panicked with the raw error. Happy to re-panic the unwrapped cause for those two if you'd rather preserve their exact original payload.

Internal callers are off the deprecated API: pqarrow already uses WriteBatchSpacedWithError, and the spaced-write test helper in internal/testutils was migrated too. The spaced-write failure tests now assert the deprecated methods panic with the underlying error.

[zeroshade]

Pushed f4b92fc5 to address the panic-payload nuance I mentioned above.

The deprecated WriteBatchSpaced / WriteBitmapBatchSpaced now re-panic the unwrapped cause (via a small shared panicCause helper) instead of panic(err), so the byte-array / FLBA paths surface the original error value again rather than the FormatRecoveredError("unknown error type", …) wrapper they'd otherwise inherit from delegating through …WithError. errors.Is(…, original) still resolves, and non-error panics fall back to the wrapper.

Net result: those two paths keep their exact pre-deprecation panic payload, while the numeric/boolean paths still fail loud instead of silently dropping the error. go build/vet/staticcheck are clean and the parquet/file + parquet/pqarrow suites pass.