Panic while writing structs with dynamic map column
gernest opened this issue · 2 comments
gernest commented
Calling (*Table).Write
with struct that has map field defined as a dynamic column triggers a panic
Here is a test case to reproduce
func TestTable_write_struct_with_map_field(t *testing.T) {
type SampleMap struct {
Labels map[string]string
}
schema := &schemapb.Schema{
Name: "test",
Columns: []*schemapb.Column{
{
Name: "labels",
StorageLayout: &schemapb.StorageLayout{
Type: schemapb.StorageLayout_TYPE_STRING,
Nullable: true,
Encoding: schemapb.StorageLayout_ENCODING_RLE_DICTIONARY,
},
Dynamic: true,
},
},
}
store, err := New()
require.Nil(t, err)
defer store.Close()
db, err := store.DB(context.Background(), "tes")
require.Nil(t, err)
table, err := db.Table("test", NewTableConfig(schema))
require.Nil(t, err)
_, err = table.Write(context.Background(),
SampleMap{
Labels: map[string]string{
"label1": "value1",
"label2": "value2",
},
},
SampleMap{
Labels: map[string]string{
"label1": "value1",
"label2": "value2",
"label3": "value3",
},
},
SampleMap{
Labels: map[string]string{
"label1": "value1",
"label2": "value2",
},
},
)
require.Nil(t, err)
}
Here is the stacktrace
--- FAIL: TestTable_write_struct_with_map_field (0.00s)
panic: runtime error: index out of range [3] with length 3 [recovered]
panic: runtime error: index out of range [3] with length 3
goroutine 7 [running]:
testing.tRunner.func1.2({0x1a0a460, 0xc0000ca078})
/usr/local/Cellar/go/1.21.4/libexec/src/testing/testing.go:1545 +0x238
testing.tRunner.func1()
/usr/local/Cellar/go/1.21.4/libexec/src/testing/testing.go:1548 +0x397
panic({0x1a0a460?, 0xc0000ca078?})
/usr/local/Cellar/go/1.21.4/libexec/src/runtime/panic.go:914 +0x21f
github.com/parquet-go/parquet-go.(*Buffer).WriteRows(0xc0002f43f0, {0xc00007f860?, 0x3, 0x0?})
/Users/g.ernest/go/pkg/mod/github.com/parquet-go/parquet-go@v0.20.0/buffer.go:392 +0x418
github.com/polarsignals/frostdb/dynparquet.(*Buffer).WriteRows(...)
/Volumes/code/frostdb/dynparquet/schema.go:960
github.com/polarsignals/frostdb/dynparquet.ValuesToBuffer(0xc0002f41b0, {0xc000151f28, 0x3, 0x120?})
/Volumes/code/frostdb/dynparquet/schema.go:1141 +0x3d8
github.com/polarsignals/frostdb.(*Table).Write(0xc00047eaa0, {0x1bd33f0, 0x235d6c0}, {0xc000151f28?, 0x0?, 0x0?})
/Volumes/code/frostdb/table.go:550 +0x7e
github.com/polarsignals/frostdb.TestTable_write_struct_with_map_field(0x0?)
/Volumes/code/frostdb/db_test.go:2280 +0x505
testing.tRunner(0xc00017ed00, 0x1aca4e0)
/usr/local/Cellar/go/1.21.4/libexec/src/testing/testing.go:1595 +0xff
created by testing.(*T).Run in goroutine 1
/usr/local/Cellar/go/1.21.4/libexec/src/testing/testing.go:1648 +0x3ad
exit status 2
FAIL github.com/polarsignals/frostdb 0.914s
gernest commented
@thorfour (*Table)Write
creates parquet rows then converts them to arrow.Record
since we have merged the new api that builds directly to arrow.Record
I think we should deprecate this api.
I confirmed parca
doesn't use (*Table)Write
I think its better to invest more time into massaging (*Table)InsertRecord
. Now we can document and iron out bits about tables and how to correctly use them.
thorfour commented
Agreed, we should just delete that API entirely now.