zeroshade commented on code in PR #1190:
URL: https://github.com/apache/arrow-go/pull/1190#discussion_r3864394961
##########
arrow/csv/writer.go:
##########
@@ -78,22 +78,27 @@ func (w *Writer) Write(record arrow.RecordBatch) error {
}
}
- recs := make([][]string, record.NumRows())
- for i := range recs {
- recs[i] = make([]string, record.NumCols())
- }
-
+ columns := make([][]string, record.NumCols())
for j, col := range record.Columns() {
rows, err := w.transformColToStringArr(w.schema.Field(j).Type,
col, w.stringReplacer)
if err != nil {
return err
}
- for i, row := range rows {
- recs[i][j] = row
+ columns[j] = rows
Review Comment:
**Blocking:** Retaining each converter result until all columns are
transformed introduces an undocumented slice-lifetime requirement for
`WithCustomTypeConverter`. A converter can currently reuse one scratch
`[]string` across callback invocations because the old implementation copied
each result immediately. With two Int32 columns `[1,2]` and `[10,20]`, such a
converter produces `10,10 / 20,20` on this branch instead of `1,10 / 2,20`; the
same probe passes on the merge base. Please preserve the previous behavior—e.g.
copy handled custom-converter results before invoking it for the next
column—and add a regression test.
--
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]
