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 64489d46 fix(parquet/variant): preserve metadata keys when cloning
(#1080)
64489d46 is described below
commit 64489d4638276c7f044953885a6d14358292d2b2
Author: Minh Vu <[email protected]>
AuthorDate: Fri Aug 7 17:44:26 2026 +0200
fix(parquet/variant): preserve metadata keys when cloning (#1080)
### Rationale for this change
Cloning Variant metadata copies the key slice headers, but the keys can
still point into the original metadata buffer. Mutating the source bytes
can then change lookups on the clone.
### What changes are included in this PR?
Clone the metadata bytes and rebuild the parsed key slices against the
cloned buffer so KeyAt and IdFor do not depend on the source.
### Are these changes tested?
- `go test ./parquet/variant`
- Added coverage for source-byte mutation and cloned key lookups.
### Are there any user-facing changes?
No API changes. This corrects the reported behavior while preserving the
existing ownership and compatibility contracts.
---
parquet/variant/variant.go | 17 +++++++++++------
parquet/variant/variant_test.go | 37 +++++++++++++++++++++++++++++++++++++
2 files changed, 48 insertions(+), 6 deletions(-)
diff --git a/parquet/variant/variant.go b/parquet/variant/variant.go
index 2eec9602..70222234 100644
--- a/parquet/variant/variant.go
+++ b/parquet/variant/variant.go
@@ -168,13 +168,18 @@ func NewMetadata(data []byte) (Metadata, error) {
// Clone creates a deep copy of the metadata.
func (m *Metadata) Clone() Metadata {
- return Metadata{
- data: bytes.Clone(m.data),
- // shallow copy of the values, but the slice is copied
- // more efficient, and nothing should be mutating the keys
- // so it's probably safe, but something we should keep in mind
- keys: slices.Clone(m.keys),
+ clone := Metadata{data: bytes.Clone(m.data)}
+ if len(clone.data) > 0 && len(m.keys) > 0 {
+ if err := clone.loadDictionary(clone.OffsetSize()); err == nil {
+ return clone
+ }
+ }
+
+ clone.keys = make([][]byte, len(m.keys))
+ for i, key := range m.keys {
+ clone.keys[i] = bytes.Clone(key)
}
+ return clone
}
func (m *Metadata) loadDictionary(offsetSz uint8) error {
diff --git a/parquet/variant/variant_test.go b/parquet/variant/variant_test.go
index e8216013..65d97000 100644
--- a/parquet/variant/variant_test.go
+++ b/parquet/variant/variant_test.go
@@ -17,6 +17,7 @@
package variant_test
import (
+ "bytes"
"encoding/json"
"fmt"
"math"
@@ -138,6 +139,42 @@ func TestMetadataEmptyKey(t *testing.T) {
assert.Equal(t, []uint32{0}, metadata.IdFor(""))
}
+func TestMetadataCloneOwnsKeys(t *testing.T) {
+ var b variant.Builder
+ start := b.Offset()
+ fields := []variant.FieldEntry{b.NextField(start, "key")}
+ require.NoError(t, b.AppendNull())
+ require.NoError(t, b.FinishObject(start, fields))
+ value, err := b.Build()
+ require.NoError(t, err)
+
+ metadataBytes := bytes.Clone(value.Metadata().Bytes())
+ metadata, err := variant.NewMetadata(metadataBytes)
+ require.NoError(t, err)
+ clone := metadata.Clone()
+ clonedBytes := bytes.Clone(clone.Bytes())
+ keyOffset := bytes.Index(metadataBytes, []byte("key"))
+ require.NotEqual(t, -1, keyOffset)
+ copy(metadataBytes[keyOffset:], "bad")
+
+ key, err := clone.KeyAt(0)
+ require.NoError(t, err)
+ assert.Equal(t, "key", key)
+ assert.Equal(t, []uint32{0}, clone.IdFor("key"))
+ assert.Equal(t, clonedBytes, clone.Bytes())
+
+ copy(metadataBytes[keyOffset:], "key")
+ clone = metadata.Clone()
+ copy(clone.Bytes()[keyOffset:], "bad")
+ key, err = clone.KeyAt(0)
+ require.NoError(t, err)
+ assert.Equal(t, "bad", key)
+ assert.Equal(t, []uint32{0}, clone.IdFor("bad"))
+ key, err = metadata.KeyAt(0)
+ require.NoError(t, err)
+ assert.Equal(t, "key", key)
+}
+
func loadVariant(t *testing.T, test string) variant.Value {
dir := getVariantDir()
if dir == "" {
