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.git
The following commit(s) were added to refs/heads/main by this push:
new 509735ce70 GH-34603: [Go][Parquet] Problem writing dictionary with
empty strings (#34709)
509735ce70 is described below
commit 509735ce7076d74e19217e5c4671566e3e4550a3
Author: Matt Topol <[email protected]>
AuthorDate: Fri Mar 31 15:30:09 2023 -0500
GH-34603: [Go][Parquet] Problem writing dictionary with empty strings
(#34709)
### Rationale for this change
Writing a dictionary encoded column consisting of empty strings ended up
writing values that consisted of a string with a NUL character in them rather
than actually writing an empty string. This fixes that issue and also cleans
the code up a little bit in doing so.
### Are these changes tested?
A unit test is added to test for the behavior.
### Are there any user-facing changes?
Users who wrote dictionary ByteArray or FixedLenByteArray columns that
contained empty strings will see this fixed when it comes to handling those
empty strings rather than having written strings containing a single NUL
character (`\x00`).
* Closes: #34603
Authored-by: Matt Topol <[email protected]>
Signed-off-by: Matt Topol <[email protected]>
---
go/internal/hashing/xxh3_memo_table.go | 5 +++
go/parquet/internal/encoding/byte_array_encoder.go | 3 --
go/parquet/internal/encoding/decoder.go | 2 -
.../encoding/fixed_len_byte_array_encoder.go | 3 --
go/parquet/pqarrow/encode_dictionary_test.go | 43 +++++++++++++++++++++-
5 files changed, 47 insertions(+), 9 deletions(-)
diff --git a/go/internal/hashing/xxh3_memo_table.go
b/go/internal/hashing/xxh3_memo_table.go
index 787a4df34d..fe4f37f23f 100644
--- a/go/internal/hashing/xxh3_memo_table.go
+++ b/go/internal/hashing/xxh3_memo_table.go
@@ -342,6 +342,11 @@ func (b *BinaryMemoTable) GetOrInsertNull() (idx int,
found bool) {
// helper function to get the offset into the builder data for a given
// index value.
func (b *BinaryMemoTable) findOffset(idx int) uintptr {
+ if b.builder.DataLen() == 0 {
+ // only empty strings, short circuit
+ return 0
+ }
+
val := b.builder.Value(idx)
for len(val) == 0 {
idx++
diff --git a/go/parquet/internal/encoding/byte_array_encoder.go
b/go/parquet/internal/encoding/byte_array_encoder.go
index 70e85569d0..52a29e5471 100644
--- a/go/parquet/internal/encoding/byte_array_encoder.go
+++ b/go/parquet/internal/encoding/byte_array_encoder.go
@@ -95,9 +95,6 @@ func (enc *DictByteArrayEncoder) WriteDict(out []byte) {
// PutByteArray adds a single byte array to buffer, updating the dictionary
// and encoded size if it's a new value
func (enc *DictByteArrayEncoder) PutByteArray(in parquet.ByteArray) {
- if in == nil {
- in = empty[:]
- }
memoIdx, found, err := enc.memo.GetOrInsert(in)
if err != nil {
panic(err)
diff --git a/go/parquet/internal/encoding/decoder.go
b/go/parquet/internal/encoding/decoder.go
index 382e8ea2ae..3c40ae7547 100644
--- a/go/parquet/internal/encoding/decoder.go
+++ b/go/parquet/internal/encoding/decoder.go
@@ -198,8 +198,6 @@ func (d *dictDecoder) DecodeIndicesSpaced(numValues,
nullCount int, validBits []
return n, nil
}
-var empty = [1]byte{0}
-
// spacedExpand is used to take a slice of data and utilize the bitmap
provided to fill in nulls into the
// correct slots according to the bitmap in order to produce a fully expanded
result slice with nulls
// in the correct slots.
diff --git a/go/parquet/internal/encoding/fixed_len_byte_array_encoder.go
b/go/parquet/internal/encoding/fixed_len_byte_array_encoder.go
index a113abc1f9..09c2a891cd 100644
--- a/go/parquet/internal/encoding/fixed_len_byte_array_encoder.go
+++ b/go/parquet/internal/encoding/fixed_len_byte_array_encoder.go
@@ -84,9 +84,6 @@ func (enc *DictFixedLenByteArrayEncoder) WriteDict(out
[]byte) {
// Put writes fixed length values to a dictionary encoded column
func (enc *DictFixedLenByteArrayEncoder) Put(in []parquet.FixedLenByteArray) {
for _, v := range in {
- if v == nil {
- v = empty[:]
- }
memoIdx, found, err := enc.memo.GetOrInsert(v)
if err != nil {
panic(err)
diff --git a/go/parquet/pqarrow/encode_dictionary_test.go
b/go/parquet/pqarrow/encode_dictionary_test.go
index 5762711a00..d56f1bdde9 100644
--- a/go/parquet/pqarrow/encode_dictionary_test.go
+++ b/go/parquet/pqarrow/encode_dictionary_test.go
@@ -103,7 +103,7 @@ func (ad *ArrowWriteDictionarySuite)
TestStatisticsWithFallback() {
}
testIndices := []arrow.Array{
- // ["b", null, "a", "b", null, "a"]
+ // ["b", null, "a", "b", null, "a"]
ad.fromJSON(mem, arrow.PrimitiveTypes.Int32, `[0, null, 3, 0,
null, 3]`),
// ["b", "c", null, "b", "c", null]
ad.fromJSON(mem, arrow.PrimitiveTypes.Int32, `[0, 1, null, 0,
1, null]`),
@@ -704,3 +704,44 @@ func TestArrowWriteNestedSubfieldDictionary(t *testing.T) {
assert.Truef(t, array.TableEqual(tbl, actual), "expected: %s\ngot: %s",
tbl, actual)
}
+
+func TestDictOfEmptyStringsRoundtrip(t *testing.T) {
+ mem := memory.NewCheckedAllocator(memory.DefaultAllocator)
+ defer mem.AssertSize(t, 0)
+
+ schema := arrow.NewSchema([]arrow.Field{
+ {Name: "reserved1", Type: arrow.BinaryTypes.String, Nullable:
true},
+ }, nil)
+
+ bldr := array.NewStringBuilder(mem)
+ defer bldr.Release()
+
+ for i := 0; i < 6; i++ {
+ bldr.AppendEmptyValue()
+ }
+
+ arr := bldr.NewArray()
+ defer arr.Release()
+ col1 := arrow.NewColumnFromArr(schema.Field(0), arr)
+ defer col1.Release()
+ tbl := array.NewTable(schema, []arrow.Column{col1}, 6)
+ defer tbl.Release()
+
+ var buf bytes.Buffer
+ require.NoError(t, pqarrow.WriteTable(tbl, &buf, 6,
+
parquet.NewWriterProperties(parquet.WithDictionaryDefault(true)),
+ pqarrow.NewArrowWriterProperties()))
+
+ result, err := pqarrow.ReadTable(context.Background(),
bytes.NewReader(buf.Bytes()), nil, pqarrow.ArrowReadProperties{}, mem)
+ require.NoError(t, err)
+ defer result.Release()
+
+ assert.EqualValues(t, 6, result.NumRows())
+ assert.EqualValues(t, 1, result.NumCols())
+ col := result.Column(0).Data().Chunk(0)
+ assert.Equal(t, arrow.STRING, col.DataType().ID())
+
+ for i := 0; i < 6; i++ {
+ assert.Zero(t, col.(*array.String).Value(i))
+ }
+}
