This is an automated email from the ASF dual-hosted git repository.
zeroshade pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-go.git
The following commit(s) were added to refs/heads/main by this push:
new 07836b1d fix(parquet): guard byte-stream-split encoder release against
never-flushed buffer (#1273)
07836b1d is described below
commit 07836b1df187a2b564b3be624dc96ff8fb30706b
Author: Tom Frank <[email protected]>
AuthorDate: Wed Sep 2 19:48:49 2026 +0300
fix(parquet): guard byte-stream-split encoder release against never-flushed
buffer (#1273)
### Rationale for this change
`byteStreamSplitEncoder.Release` unconditionally passes
`enc.flushBuffer` to `releaseBufferToPool`, which dereferences it. The
flush buffer is allocated lazily on the first `FlushValues`, so
releasing an encoder that never flushed — which `columnWriter.Close`
does for a column chunk that received no values — dereferences nil and
crashes the process with a SIGSEGV.
`ByteStreamSplitFixedLenByteArrayEncoder.Release` has the same defect.
### What changes are included in this PR?
A nil guard at the top of `releaseBufferToPool`, covering both encoders,
plus a regression test that releases a never-flushed BYTE_STREAM_SPLIT
encoder for all five physical types (INT32, INT64, FLOAT, DOUBLE,
FIXED_LEN_BYTE_ARRAY).
### Are these changes tested?
Yes. The new test panics on every subtest without the fix and passes
with it; the existing `parquet/internal/encoding` and `parquet/file`
suites pass.
### Are there any user-facing changes?
No API changes. Writing a parquet file with a BYTE_STREAM_SPLIT column
no longer panics when a column chunk ends up empty (e.g. an empty
trailing row group).
Co-authored-by: Claude Fable 5 <[email protected]>
---
parquet/internal/encoding/byte_stream_split.go | 6 +++
.../encoding/byte_stream_split_release_test.go | 52 ++++++++++++++++++++++
2 files changed, 58 insertions(+)
diff --git a/parquet/internal/encoding/byte_stream_split.go
b/parquet/internal/encoding/byte_stream_split.go
index 970e0f88..d5820093 100644
--- a/parquet/internal/encoding/byte_stream_split.go
+++ b/parquet/internal/encoding/byte_stream_split.go
@@ -94,6 +94,12 @@ var (
)
func releaseBufferToPool(pooled *PooledBufferWriter) {
+ // the byte-stream-split encoders allocate their flush buffer lazily on
the first
+ // FlushValues, so releasing an encoder that never flushed (e.g. a
column chunk that
+ // received no values) passes nil here
+ if pooled == nil {
+ return
+ }
buf := pooled.buf
memory.Set(buf.Buf(), 0)
buf.ResizeNoShrink(0)
diff --git a/parquet/internal/encoding/byte_stream_split_release_test.go
b/parquet/internal/encoding/byte_stream_split_release_test.go
new file mode 100644
index 00000000..0b678787
--- /dev/null
+++ b/parquet/internal/encoding/byte_stream_split_release_test.go
@@ -0,0 +1,52 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package encoding
+
+import (
+ "testing"
+
+ "github.com/apache/arrow-go/v18/arrow/memory"
+ "github.com/apache/arrow-go/v18/parquet"
+ "github.com/apache/arrow-go/v18/parquet/schema"
+ "github.com/stretchr/testify/assert"
+)
+
+// The byte-stream-split encoders allocate their flush buffer lazily on the
first
+// FlushValues, so an encoder released without ever flushing (as
columnWriter.Close
+// does for a column chunk that received no values) must not dereference the
nil buffer.
+func TestByteStreamSplitReleaseWithoutFlush(t *testing.T) {
+ tests := []struct {
+ name string
+ typ parquet.Type
+ typeLen int32
+ }{
+ {"int32", parquet.Types.Int32, -1},
+ {"int64", parquet.Types.Int64, -1},
+ {"float32", parquet.Types.Float, -1},
+ {"float64", parquet.Types.Double, -1},
+ {"fixed_len_byte_array", parquet.Types.FixedLenByteArray, 4},
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ descr :=
schema.NewColumn(schema.MustPrimitive(schema.NewPrimitiveNode(
+ "col", parquet.Repetitions.Required, tt.typ,
-1, tt.typeLen)), 0, 0)
+ enc := NewEncoder(tt.typ,
parquet.Encodings.ByteStreamSplit, false, descr, memory.DefaultAllocator)
+ assert.NotPanics(t, enc.Release)
+ })
+ }
+}
