[vuanhphung]

What did you change, and why?

Why: Rendering nested ARRAY/MAP/STRUCT values currently boxes each Boolean or integer leaf and calls json.Marshal, adding allocations during kernel-backend row conversion.

Change: Write Boolean literals and signed integers directly into the existing JSON builder, using a stack buffer for integer formatting. Extend cross-backend rendering parity tests and add a changelog entry.

How do you know it works?

Testing: Scanner and Thrift/kernel parity suites pass through temporary Bazel adapters (Go 1.27.3, Arrow v12). The added sliced STRUCT case compares Boolean and all signed integer widths against Thrift, including bounds, zero/negative values, and parent/child nulls. Focused golangci-lint reports 0 new issues.

Performance: Live DBSQL on pecotesting, released v1.18.0 vs. v1.18.0 with only this formatting patch (same kernel bindings v1.3.0). Medians across six balanced AB/BA pairs per case, using 2 million synthetic rows with equivalent ARRAY/MAP/STRUCT shapes. Timed query execution and complete Rows.Next/Scan drain; connection setup excluded.

Dashboard testcase	v1.18.0	+ formatting patch	Time reduction
extended_types_array_cloudfetch	2.264 s	1.624 s	28.3%
extended_types_array_cloudfetch_qrc	1.923 s	1.187 s	38.3%
extended_types_map_cloudfetch	2.793 s	2.357 s	15.6%
extended_types_map_cloudfetch_qrc	2.297 s	1.886 s	17.9%
extended_types_struct_cloudfetch	2.122 s	1.889 s	11.0%
extended_types_struct_cloudfetch_qrc	1.729 s	1.544 s	10.7%

Each timed run returned 2 million rows; 1,000 ordered non-null output samples per type matched between builds. CloudFetch and QRC behavior were verified from statement metadata. Temporary fixtures were deleted after measurement. These are synthetic-fixture results, not measurements of the unavailable dashboard dataset or Reyden. Top-level Boolean conversion is unchanged.

[peco-review-bot]

Verdict: 1 Nit

Looks good, no blocking concerns. The new Boolean and Int8–Int64 branches in writeJSON produce the same bytes as the json.Marshal path they replace. Nulls are still handled before the type switch. The new tests cover limit values, parent/child nulls and sliced arrays, with and without the struct-key cache.

Other findings

• ⚪ Nit — writeIntJSON can never fail, but it still returns an error. It could return nothing, and the callers could then return nil themselves. That would make it clearer that no error can come from this path.

[peco-review-bot]

Verdict: 1 Nit

Looks good. The new Boolean and Int8–Int64 fast paths produce the same bytes as json.Marshal, nulls are still handled by the IsNull check before the switch, and the tests cover the sliced and offset cases, the min/max values for each width, and both the cached and uncached struct-key paths. I found nothing beyond one stylistic nit.

Other findings

• ⚪ Nit — writeIntJSON always returns nil. It could return nothing, with callers doing writeIntJSON(...); return nil, matching the inline Boolean arm. That would make it clear the function can't fail. Purely stylistic.

[peco-review-bot]

Verdict: 1 Low

Looks good. Output for nested Boolean and Int8–Int64 leaves is byte-identical to the old json.Marshal path: strconv.AppendInt base-10 matches encoding/json, and 20 bytes is enough for math.MinInt64. The new sliced-struct parity case covers the bounds and parent/child nulls. My one low note: nothing in the tests guards the allocation reduction itself.

[peco-review-bot]

🔵 Low — The PR's main claim is fewer allocations, but no test or benchmark checks it. The parity case only shows the output still matches Thrift, so it would keep passing if a later edit put Boolean/int leaves back on scalarForJSON + json.Marshal. Consider adding a small testing.AllocsPerRun test or a Benchmark* in internal/arrowscan that renders an ARRAY<INT>/STRUCT<bool,int64> cell, so the allocation drop is measured and protected.

[aakash-saravanan-db]

LGTM. Scanner and Arrow parity suites passed locally; no blocking code findings.