MINOR: [Go] Normalize zero-length C Data buffers with regression coverage

go/arrow/cdata/cdata.go

@@ -359,10 +359,10 @@ func (imp *cimporter) importChild(parent *cimporter, src *CArrowArray) error {
 
 // import any child arrays for lists, structs, and so on.
 func (imp *cimporter) doImportChildren() error {
-	children := unsafe.Slice(imp.arr.children, imp.arr.n_children)
-
-	if len(children) > 0 {
-		imp.children = make([]cimporter, len(children))
+	var children []*CArrowArray
+	if imp.arr.n_children > 0 {
+		children = unsafe.Slice(imp.arr.children, imp.arr.n_children)
+		imp.children = make([]cimporter, imp.arr.n_children)
 	}
 
 	// handle the cases
@@ -701,10 +701,12 @@ func (imp *cimporter) importBinaryViewLike() (err error) {
 		return
 	}
 
-	dataBufferSizes := unsafe.Slice((*int64)(unsafe.Pointer(imp.cbuffers[len(buffers)])), len(buffers)-2)
-	for i, size := range dataBufferSizes {
-		if buffers[i+2], err = imp.importVariableValuesBuffer(i+2, 1, size); err != nil {
-			return
+	if len(buffers) > 2 {
+		dataBufferSizes := unsafe.Slice((*int64)(unsafe.Pointer(imp.cbuffers[len(buffers)])), len(buffers)-2)
+		for i, size := range dataBufferSizes {
+			if buffers[i+2], err = imp.importVariableValuesBuffer(i+2, 1, size); err != nil {
+				return
+			}
 		}
 	}
 
@@ -856,7 +858,7 @@ func (imp *cimporter) checkNumBuffers(n int64) error {
 func (imp *cimporter) importBuffer(bufferID int, sz int64) (*memory.Buffer, error) {
 	// this is not a copy, we're just having a slice which points at the data
 	// it's still owned by the C.ArrowArray object and its backing C++ object.
-	if imp.cbuffers[bufferID] == nil {
+	if imp.cbuffers[bufferID] == nil || sz == 0 {
 		if sz != 0 {
 			return nil, errors.New("invalid buffer")
 		}

go/arrow/cdata/cdata_fulltest.c

@@ -335,9 +335,29 @@ void export_int32_array(const int32_t*, int64_t, struct ArrowArray*);
 
 static void release_str_array(struct ArrowArray* array) {
     assert(array->n_buffers == 3);
+    if (array->buffers[0] != NULL) {
+        free((void*) array->buffers[0]);
+    }
+    if (array->buffers[1] != NULL && array->buffers[1] != (void*)0x1) {
+        free((void*) array->buffers[1]);
+    }
+    if (array->buffers[2] != NULL && array->buffers[2] != (void*)0x1) {
+        free((void*) array->buffers[2]);
+    }
+    free(array->buffers);
+    array->release = NULL;
+}
+
+static void release_list_array(struct ArrowArray* array) {
+    assert(array->n_buffers == 2);
+    if (array->buffers[0] != NULL) {
+        free((void*) array->buffers[0]);
+    }
     free((void*) array->buffers[1]);
-    free((void*) array->buffers[2]);
     free(array->buffers);
+    ArrowArrayRelease(array->children[0]);
+    free(array->children[0]);
+    free(array->children);
     array->release = NULL;
 }
 
@@ -361,6 +381,192 @@ void export_str_array(const char* data, const int32_t* offsets, int64_t nitems,
     out->buffers[2] = data;
 }
 
+void export_str_array_with_nulls(int64_t nitems, struct ArrowArray* out) {
+    *out = (struct ArrowArray) {
+        .length = nitems,
+        .offset = 0,
+        .null_count = nitems,
+        .n_buffers = 3,
+        .n_children = 0,
+        // The C Data Interface permits arbitrary pointer values when
+        // n_children is zero. Keep a non-null sentinel here so the importer
+        // regression test exercises that contract as well.
+        .children = (struct ArrowArray**)0x1,
+        .dictionary = NULL,
+        // bookkeeping
+        .release = &release_str_array
+    };
+
+    out->buffers = (const void**)malloc(sizeof(void*) * out->n_buffers);
+    assert(out->buffers != NULL);
+    int64_t bitmap_nbytes = (nitems + 7) / 8;
+    out->buffers[0] = malloc(bitmap_nbytes);
+    memset((void*)out->buffers[0], 0, bitmap_nbytes);
+    out->buffers[1] = malloc((nitems + 1) * sizeof(int32_t));
+    memset((void*)out->buffers[1], 0, (nitems + 1) * sizeof(int32_t));
+    out->buffers[2] = (void*)0x1;
+}
+
+void export_str_array_with_empty_values(int64_t nitems, struct ArrowArray* out) {
+    *out = (struct ArrowArray) {
+        .length = nitems,
+        .offset = 0,
+        .null_count = 0,
+        .n_buffers = 3,
+        .n_children = 0,
+        .children = (struct ArrowArray**)0x1,
+        .dictionary = NULL,
+        .release = &release_str_array
+    };
+
+    out->buffers = (const void**)malloc(sizeof(void*) * out->n_buffers);
+    assert(out->buffers != NULL);
+    out->buffers[0] = NULL;
+    out->buffers[1] = malloc((nitems + 1) * sizeof(int32_t));
+    memset((void*)out->buffers[1], 0, (nitems + 1) * sizeof(int32_t));
+    out->buffers[2] = (void*)0x1;
+}
+
+void export_large_str_array_with_nulls(int64_t nitems, struct ArrowArray* out) {
+    *out = (struct ArrowArray) {
+        .length = nitems,
+        .offset = 0,
+        .null_count = nitems,
+        .n_buffers = 3,
+        .n_children = 0,
+        .children = (struct ArrowArray**)0x1,
+        .dictionary = NULL,
+        .release = &release_str_array
+    };
+
+    out->buffers = (const void**)malloc(sizeof(void*) * out->n_buffers);
+    assert(out->buffers != NULL);
+    int64_t bitmap_nbytes = (nitems + 7) / 8;
+    out->buffers[0] = malloc(bitmap_nbytes);
+    memset((void*)out->buffers[0], 0, bitmap_nbytes);
+    out->buffers[1] = malloc((nitems + 1) * sizeof(int64_t));
+    memset((void*)out->buffers[1], 0, (nitems + 1) * sizeof(int64_t));
+    out->buffers[2] = (void*)0x1;
+}
+
+void export_large_str_array_with_empty_values(int64_t nitems, struct ArrowArray* out) {
+    *out = (struct ArrowArray) {
+        .length = nitems,
+        .offset = 0,
+        .null_count = 0,
+        .n_buffers = 3,
+        .n_children = 0,
+        .children = (struct ArrowArray**)0x1,
+        .dictionary = NULL,
+        .release = &release_str_array
+    };
+
+    out->buffers = (const void**)malloc(sizeof(void*) * out->n_buffers);
+    assert(out->buffers != NULL);
+    out->buffers[0] = NULL;
+    out->buffers[1] = malloc((nitems + 1) * sizeof(int64_t));
+    memset((void*)out->buffers[1], 0, (nitems + 1) * sizeof(int64_t));
+    out->buffers[2] = (void*)0x1;
+}
+
+void export_list_str_array_with_null_child(int64_t nitems, struct ArrowArray* out) {
+    *out = (struct ArrowArray) {
+        .length = nitems,
+        .offset = 0,
+        .null_count = 0,
+        .n_buffers = 2,
+        .n_children = 1,
+        .dictionary = NULL,
+        .release = &release_list_array
+    };
+
+    out->buffers = (const void**)malloc(sizeof(void*) * out->n_buffers);
+    assert(out->buffers != NULL);
+    out->buffers[0] = NULL;
+    out->buffers[1] = malloc((nitems + 1) * sizeof(int32_t));
+    int32_t* offsets = (int32_t*)out->buffers[1];
+    for (int64_t i = 0; i <= nitems; ++i) {
+        offsets[i] = (int32_t)i;
+    }
+
+    out->children = malloc(sizeof(struct ArrowArray*));
+    out->children[0] = malloc(sizeof(struct ArrowArray));
+    export_str_array_with_nulls(nitems, out->children[0]);
+}
+
+void export_empty_string_view(struct ArrowArray* out) {
+    *out = (struct ArrowArray) {
+        .length = 0,
+        .offset = 0,
+        .null_count = 0,
+        .n_buffers = 3,
+        .n_children = 0,
+        .children = (struct ArrowArray**)0x1,
+        .dictionary = NULL,
+        .release = &release_str_array
+    };
+
+    out->buffers = (const void**)malloc(sizeof(void*) * out->n_buffers);
+    assert(out->buffers != NULL);
+    out->buffers[0] = NULL;
+    out->buffers[1] = (void*)0x1;
+    out->buffers[2] = (void*)0x1;
+}
+
+static int all_null_string_stream_schema(struct ArrowArrayStream* st, struct ArrowSchema* out) {
+    out->children = malloc(sizeof(struct ArrowSchema*));
+    out->n_children = 1;
+    out->children[0] = malloc(sizeof(struct ArrowSchema));
+    *out->children[0] = (struct ArrowSchema) {
+        .format = "u",
+        .name = "value",
+        .metadata = NULL,
+        .flags = ARROW_FLAG_NULLABLE,
+        .children = (struct ArrowSchema**)0x1,
+        .n_children = 0,
+        .dictionary = NULL,
+        .release = &release_nested_static,
+    };
+    out->format = "+s";
+    out->release = &release_nested_static;
+    return 0;
+}
+
+static int next_all_null_string_record(struct ArrowArrayStream* st, struct ArrowArray* out) {
+    struct streamcounter* cnter = (struct streamcounter*)(st->private_data);
+    if (cnter->n == cnter->max) {
+        ArrowArrayMarkReleased(out);
+        return 0;
+    }
+    cnter->n++;
+
+    *out = (struct ArrowArray) {
+        .offset = 0,
+        .dictionary = NULL,
+        .length = 1000,
+        .null_count = 0,
+        .buffers = (const void**)malloc(sizeof(void*)),
+        .n_children = 1,
+        .n_buffers = 1,
+        .release = &release_the_array
+    };
+    out->buffers[0] = NULL;
+    out->children = malloc(sizeof(struct ArrowArray*));
+    out->children[0] = malloc(sizeof(struct ArrowArray));
+    export_str_array_with_nulls(1000, out->children[0]);
+    return 0;
+}
+
+void setup_all_null_string_stream(struct ArrowArrayStream* out) {
+    struct streamcounter* cnt = malloc(sizeof(struct streamcounter));
+    cnt->max = 1;
+    cnt->n = 0;
+    out->get_next = &next_all_null_string_record;
+    out->get_schema = &all_null_string_stream_schema;
+    out->release = &release_stream;
+    out->private_data = cnt;
+}
+
 static int next_record(struct ArrowArrayStream* st, struct ArrowArray* out) {
     struct streamcounter* cnter = (struct streamcounter*)(st->private_data);
     if (cnter->n == cnter->max) {

go/arrow/cdata/cdata_test.go

@@ -42,6 +42,7 @@ import (
 	"github.com/apache/arrow/go/v17/arrow/memory"
 	"github.com/apache/arrow/go/v17/arrow/memory/mallocator"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestSchemaExport(t *testing.T) {
@@ -710,6 +711,91 @@ func TestNestedArrays(t *testing.T) {
 	}
 }
 
+func TestStrArrayAllNulls(t *testing.T) {
+	for _, tc := range []struct {
+		name   string
+		dt     arrow.DataType
+		export func(int64) CArrowArray
+	}{
+		{name: "string", dt: arrow.BinaryTypes.String, export: exportStrArrayWithNulls},
+		{name: "binary", dt: arrow.BinaryTypes.Binary, export: exportStrArrayWithNulls},
+		{name: "large_string", dt: arrow.BinaryTypes.LargeString, export: exportLargeStrArrayWithNulls},
+		{name: "large_binary", dt: arrow.BinaryTypes.LargeBinary, export: exportLargeStrArrayWithNulls},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			arr := tc.export(1000)
+			carr, err := ImportCArrayWithType(&arr, tc.dt)
+			require.NoError(t, err)
+			defer carr.Release()
+
+			buffer := carr.Data().Buffers()[2]
+			require.NotNil(t, buffer)
+			require.Empty(t, buffer.Bytes())
+			assert.NotEqual(t, uintptr(1), uintptr(unsafe.Pointer(unsafe.SliceData(buffer.Buf()))))
+			assert.Equal(t, 1000, carr.Len())
+			assert.Equal(t, 1000, carr.NullN())
+			for i := 0; i < carr.Len(); i++ {
+				assert.True(t, carr.IsNull(i))
+			}
+		})
+	}
+}
+
+func TestStrArrayEmptyValues(t *testing.T) {
+	for _, tc := range []struct {
+		name   string
+		dt     arrow.DataType
+		export func(int64) CArrowArray
+	}{
+		{name: "string", dt: arrow.BinaryTypes.String, export: exportStrArrayWithEmptyValues},
+		{name: "binary", dt: arrow.BinaryTypes.Binary, export: exportStrArrayWithEmptyValues},
+		{name: "large_string", dt: arrow.BinaryTypes.LargeString, export: exportLargeStrArrayWithEmptyValues},
+		{name: "large_binary", dt: arrow.BinaryTypes.LargeBinary, export: exportLargeStrArrayWithEmptyValues},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			arr := tc.export(1000)
+			carr, err := ImportCArrayWithType(&arr, tc.dt)
+			require.NoError(t, err)
+			defer carr.Release()
+
+			buffer := carr.Data().Buffers()[2]
+			require.NotNil(t, buffer)
+			require.Empty(t, buffer.Bytes())
+			assert.NotEqual(t, uintptr(1), uintptr(unsafe.Pointer(unsafe.SliceData(buffer.Buf()))))
+			assert.Equal(t, 1000, carr.Len())
+			assert.Zero(t, carr.NullN())
+			for i := 0; i < carr.Len(); i++ {
+				assert.Empty(t, carr.ValueStr(i))
+			}
+		})
+	}
+}
+
+func TestListStringArrayAllNullChildValues(t *testing.T) {
+	arr := exportListStrArrayWithNullChild(1000)
+	imported, err := ImportCArrayWithType(&arr, arrow.ListOf(arrow.BinaryTypes.String))
+	require.NoError(t, err)
+	defer imported.Release()
+
+	list := imported.(*array.List)
+	assert.Equal(t, 1000, list.Len())
+	values := list.ListValues()
+	assert.Equal(t, 1000, values.NullN())
+	buffer := values.Data().Buffers()[2]
+	require.NotNil(t, buffer)
+	require.Empty(t, buffer.Bytes())
+	assert.NotEqual(t, uintptr(1), uintptr(unsafe.Pointer(unsafe.SliceData(buffer.Buf()))))
+}
+
+func TestEmptyStringViewArbitraryZeroLengthPointer(t *testing.T) {
+	arr := exportEmptyStringView()
+	imported, err := ImportCArrayWithType(&arr, arrow.BinaryTypes.StringView)
+	require.NoError(t, err)
+	defer imported.Release()
+	assert.Zero(t, imported.Len())
+	assert.IsType(t, &array.StringView{}, imported)
+}
+
 func TestRecordBatch(t *testing.T) {
 	mem := mallocator.NewMallocator()
 	defer mem.AssertSize(t, 0)
@@ -767,6 +853,33 @@ func TestRecordReaderStream(t *testing.T) {
 	}
 }
 
+func TestRecordReaderStreamAllNullStrings(t *testing.T) {
+	stream := allNullStringStreamTest()
+	defer releaseStream(stream)
+
+	rdr, err := ImportCRecordReader(stream, nil)
+	require.NoError(t, err)
+
+	rec, err := rdr.Read()
+	require.NoError(t, err)
+	require.NotNil(t, rec)
+	assert.EqualValues(t, 1000, rec.NumRows())
+	require.EqualValues(t, 1, rec.NumCols())
+	col := rec.Column(0)
+	assert.Equal(t, 1000, col.NullN())
+	for i := 0; i < col.Len(); i++ {
+		assert.True(t, col.IsNull(i))
+	}
+
+	buffer := col.Data().Buffers()[2]
+	require.NotNil(t, buffer)
+	require.Empty(t, buffer.Bytes())
+	assert.NotEqual(t, uintptr(1), uintptr(unsafe.Pointer(unsafe.SliceData(buffer.Buf()))))
+
+	_, err = rdr.Read()
+	assert.ErrorIs(t, err, io.EOF)
+}
+
 func TestExportRecordReaderStream(t *testing.T) {
 	reclist := arrdata.Records["primitives"]
 	rdr, _ := array.NewRecordReader(reclist[0].Schema(), reclist)