From 97e1fb5efe48e062c7ed83826038b9f8531c01b4 Mon Sep 17 00:00:00 2001 From: Martin Hilton Date: Thu, 7 Aug 2025 09:27:52 +0100 Subject: [PATCH] fix: panic in arrow table buffer An empty arrow.TableBuffer can panic when calling Len() if it is in the process of being appended to by a table.BufferBuilder. Normally the TableBuffer maintains the invariant that the size of Columns is the same as the size if Values so to check one is equivalent to checkint the other. However when a BufferBuilder is merging buffers with different schema it will add new null-valued columns to the TableBuffer. This checks the TableBuffer.Len() between adding the column and adding the null-valued column. In the case that the TableBuffer is empty, but the Columns array is a zero-length slice, rather than nil, a panic occurs. There are two fixes here. Either of which fix the symptom, but both are worthwhile changes. TableBuffer.Len() now checks the length of the slice it is attempting to access before reading it. BufferBuilder.normalizeSchema() now checks that the Columns slice length is 0 (rather than the value being nil) in order to use the shortcut normalize code path. --- arrow/table_buffer.go | 2 +- execute/table/buffered_builder.go | 2 +- execute/table/buffered_builder_test.go | 41 ++++++++++++++++++++++++++ 3 files changed, 43 insertions(+), 2 deletions(-) diff --git a/arrow/table_buffer.go b/arrow/table_buffer.go index 7952cf2d18..e681a4ebb7 100644 --- a/arrow/table_buffer.go +++ b/arrow/table_buffer.go @@ -26,7 +26,7 @@ type TableBuffer struct { var _ flux.ColReader = (*TableBuffer)(nil) func (t *TableBuffer) Len() int { - if len(t.Columns) == 0 { + if len(t.Values) == 0 { return 0 } return t.Values[0].Len() diff --git a/execute/table/buffered_builder.go b/execute/table/buffered_builder.go index 6ce5a30298..ea65c10f5c 100644 --- a/execute/table/buffered_builder.go +++ b/execute/table/buffered_builder.go @@ -89,7 +89,7 @@ func (b *BufferedBuilder) appendBuffer(cr flux.ColReader, mem memory.Allocator) func (b *BufferedBuilder) normalizeTableSchema(cols []flux.ColMeta, mem memory.Allocator) error { // If there are no columns set for this builder, inherit the ones // that were passed in. - if b.Columns == nil { + if len(b.Columns) == 0 { b.Columns = cols return nil } diff --git a/execute/table/buffered_builder_test.go b/execute/table/buffered_builder_test.go index f3f8078b71..cf1141a00c 100644 --- a/execute/table/buffered_builder_test.go +++ b/execute/table/buffered_builder_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/influxdata/flux" + "github.com/influxdata/flux/execute" "github.com/influxdata/flux/execute/table" "github.com/influxdata/flux/execute/table/static" "github.com/influxdata/flux/memory" @@ -183,3 +184,43 @@ func TestBufferedBuilder_AppendTable(t *testing.T) { }) } } + +func TestBufferedBuilder_AppendTable_EmptyTable(t *testing.T) { + mem := memory.DefaultAllocator + + gk := execute.NewGroupKey(nil, nil) + emptyTable := execute.NewEmptyTable(gk, []flux.ColMeta{}) + b := table.NewBufferedBuilder(gk, mem) + + // Append an empty table to the builder. + err := b.AppendTable(emptyTable) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + in := static.TableGroup{ + static.Table{ + static.Times("_time", "2020-01-01T00:00:00Z", 10, 20, 30, 40, 50), + static.Floats("_value", 3, 8, 2, 4, 11, 6), + }, + } + err = in.Do(func(tbl flux.Table) error { + return b.AppendTable(tbl) + }) + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + want := static.Table{ + static.Times("_time", "2020-01-01T00:00:00Z", 10, 20, 30, 40, 50), + static.Floats("_value", 3, 8, 2, 4, 11, 6), + } + + out, err := b.Table() + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + if diff := table.Diff(want, table.Iterator{out}); diff != "" { + t.Fatalf("unexpected diff -want/+got:\n%s", diff) + } +}