zeroshade opened a new pull request, #1400: URL: https://github.com/apache/arrow-go/pull/1400
### Rationale for this change `deltaBitPackEncoder.FlushValues` hands the sink's bytes to the caller but left the bit writer (and the first/current value) in place. The next `Put` replaces the writer, but a page whose values are all null calls `Put` with nothing, so its flush still used the old writer's byte count and copied that many bytes out of the fresh sink buffer: - **Through a column writer** (pqarrow): no crash and the data reads back correctly, but an all-null page is padded with the previous page's length in zeros, and the stale `EstimatedDataEncodedSize` makes the writer cut pages early. A nullable INT32 column of 20,000 random values, 5,000 nulls and 10 values, written with 1 KB pages, came out at 92,002 bytes / 25 pages on main versus 82,954 bytes / 21 pages with this change. - **Using the encoder directly**: after a large page, the next all-null flush panics (`slice bounds out of range [:96061] with capacity 2048`). Found while reviewing #1396. ### What changes are included in this PR? `FlushValues` drops the bit writer and resets the first/current value after flushing, so an empty page is exactly the header a fresh encoder writes and the size estimate is 0 until the next `Put`. No API change. ### Are these changes tested? Yes. `TestDeltaBitPackedReuseAfterAllNullPage` (in `encoding_test.go`, next to the other delta tests) writes a first page of 3, 1,000 or 100,000 values and then checks that an all-null page is byte-identical to a fresh encoder's, that the size estimate is 0, and that the following page still decodes. It fails on main (wrong bytes and estimate, then a panic) and passes with the change. - `go test ./parquet/internal/encoding ./parquet/file ./parquet/pqarrow`: ok - `go test -tags noasm` and `GOARCH=386 go test` for `./parquet/internal/encoding`: ok; delta tests pass under `-race` - `go vet ./parquet/internal/encoding` and `gofmt -l`: clean This touches the same file as #1394 but different functions; the two merge cleanly. ### Are there any user-facing changes? No API changes. Delta-encoded columns with all-null pages are written smaller and with correctly sized pages. -- 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]
