This is an automated email from the ASF dual-hosted git repository.
lidavidm pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-adbc.git
The following commit(s) were added to refs/heads/main by this push:
new 6747003deb fix(c/driver/postgresql): check child nulls in ingest
(#4848)
6747003deb is described below
commit 6747003deb76eaf2d90d2c4ad5095b64505adf8c
Author: David Li <[email protected]>
AuthorDate: Sun Oct 4 16:31:27 2026 -0700
fix(c/driver/postgresql): check child nulls in ingest (#4848)
Closes #4846.
Generated-by: GPT-6 Astra
---
.../postgresql/copy/postgres_copy_writer_test.cc | 148 +++++++++++++++
c/driver/postgresql/copy/writer.h | 17 +-
.../queries/ingest/fixed_size_list.txtcase | 201 +++++++++++++++++++++
.../validation/queries/ingest/list.txtcase | 80 ++++++++
compose.yaml | 2 +-
5 files changed, 445 insertions(+), 3 deletions(-)
diff --git a/c/driver/postgresql/copy/postgres_copy_writer_test.cc
b/c/driver/postgresql/copy/postgres_copy_writer_test.cc
index e3dcb2966a..801489efa9 100644
--- a/c/driver/postgresql/copy/postgres_copy_writer_test.cc
+++ b/c/driver/postgresql/copy/postgres_copy_writer_test.cc
@@ -1837,6 +1837,154 @@ TEST_F(PostgresCopyTest,
PostgresCopyWriteFixedSizeListSlicedMatchesDirect) {
}
}
+// COPY (SELECT CAST("col" AS INTEGER ARRAY) AS "col" FROM (VALUES
('{NULL,42}'),
+// ('{0,7}'), ('{NULL,NULL}'), (NULL)) AS drvd("col")) TO STDOUT WITH (FORMAT
binary);
+static const uint8_t kTestPgCopyNullIntegerArray[] = {
+ 0x50, 0x47, 0x43, 0x4f, 0x50, 0x59, 0x0a, 0xff, 0x0d, 0x0a, 0x00, 0x00,
0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x20,
0x00,
+ 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x17, 0x00,
0x00,
+ 0x00, 0x02, 0x00, 0x00, 0x00, 0x01, 0xff, 0xff, 0xff, 0xff, 0x00, 0x00,
0x00,
+ 0x04, 0x00, 0x00, 0x00, 0x2a, 0x00, 0x01, 0x00, 0x00, 0x00, 0x24, 0x00,
0x00,
+ 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x17, 0x00, 0x00,
0x00,
+ 0x02, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00,
0x00,
+ 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x07, 0x00, 0x01, 0x00, 0x00,
0x00,
+ 0x1c, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00,
0x17,
+ 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x01, 0xff, 0xff, 0xff, 0xff,
0xff,
+ 0xff, 0xff, 0xff, 0x00, 0x01, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
+
+// Regression for GH-4846: handle null elements inside lists
+TEST_F(PostgresCopyTest, PostgresCopyWriteNullListElements) {
+ for (const auto type :
+ {NANOARROW_TYPE_LIST, NANOARROW_TYPE_LARGE_LIST,
NANOARROW_TYPE_FIXED_SIZE_LIST}) {
+ for (const bool sliced : {false, true}) {
+ adbc_validation::Handle<struct ArrowSchema> schema;
+ adbc_validation::Handle<struct ArrowArray> array;
+ ASSERT_EQ(ArrowSchemaInitFromType(&schema.value, NANOARROW_TYPE_STRUCT),
0);
+ ASSERT_EQ(ArrowSchemaAllocateChildren(&schema.value, 1), 0);
+ ArrowSchemaInit(schema->children[0]);
+ if (type == NANOARROW_TYPE_FIXED_SIZE_LIST) {
+ ASSERT_EQ(ArrowSchemaSetTypeFixedSize(schema->children[0], type, 2),
0);
+ } else {
+ ASSERT_EQ(ArrowSchemaSetType(schema->children[0], type), 0);
+ }
+ ASSERT_EQ(ArrowSchemaSetName(schema->children[0], "col"), 0);
+ ASSERT_EQ(
+ ArrowSchemaSetType(schema->children[0]->children[0],
NANOARROW_TYPE_INT32), 0);
+ ASSERT_EQ(ArrowArrayInitFromSchema(&array.value, &schema.value,
nullptr), 0);
+ ASSERT_EQ(ArrowArrayStartAppending(&array.value), 0);
+ using Row = std::optional<std::vector<std::optional<int32_t>>>;
+ const std::vector<Row> rows = {
+ std::vector<std::optional<int32_t>>{std::nullopt, 42},
+ std::vector<std::optional<int32_t>>{0, 7},
+ std::vector<std::optional<int32_t>>{std::nullopt, std::nullopt},
std::nullopt};
+ auto append_row = [&](const Row& row) {
+ auto* list = array->children[0];
+ if (!row.has_value()) {
+ ASSERT_EQ(ArrowArrayAppendNull(list, 1), 0);
+ } else {
+ for (const auto& value : *row) {
+ if (value.has_value()) {
+ ASSERT_EQ(ArrowArrayAppendInt(list->children[0], *value), 0);
+ } else {
+ ASSERT_EQ(ArrowArrayAppendNull(list->children[0], 1), 0);
+ }
+ }
+ ASSERT_EQ(ArrowArrayFinishElement(list), 0);
+ }
+ ASSERT_EQ(ArrowArrayFinishElement(&array.value), 0);
+ };
+ if (sliced) {
+
ASSERT_NO_FATAL_FAILURE(append_row(std::vector<std::optional<int32_t>>{8, 9}));
+ }
+ for (const auto& row : rows) {
+ ASSERT_NO_FATAL_FAILURE(append_row(row));
+ }
+ ASSERT_EQ(ArrowArrayFinishBuildingDefault(&array.value, nullptr), 0);
+ if (sliced) {
+ array->length = rows.size();
+ array->children[0]->offset = 1;
+ array->children[0]->length = rows.size();
+ }
+
+ PostgresCopyStreamWriteTester tester;
+ ASSERT_EQ(tester.Init(&schema.value, &array.value, *type_resolver_), 0);
+ ASSERT_EQ(tester.WriteAll(nullptr), ENODATA);
+ const struct ArrowBuffer buf = tester.WriteBuffer();
+ // The last 2 bytes of a message can be transmitted via PQputCopyData
+ // so no need to test those bytes from the Writer
+ constexpr size_t buf_size = sizeof(kTestPgCopyNullIntegerArray) - 2;
+ ASSERT_EQ(buf.size_bytes, buf_size);
+ for (size_t i = 0; i < buf_size; i++) {
+ ASSERT_EQ(buf.data[i], kTestPgCopyNullIntegerArray[i])
+ << "failure at index " << i;
+ }
+ }
+ }
+}
+
+TEST_F(PostgresCopyTest, PostgresCopyWriteNullDictionaryListElements) {
+ for (const auto type : {NANOARROW_TYPE_STRING, NANOARROW_TYPE_BINARY}) {
+ SCOPED_TRACE(type);
+ std::vector<uint8_t> reference;
+ for (const bool dictionary : {false, true}) {
+ adbc_validation::Handle<struct ArrowSchema> schema;
+ adbc_validation::Handle<struct ArrowArray> array;
+ ASSERT_EQ(
+ adbc_validation::MakeSchema(
+ &schema.value, {adbc_validation::SchemaField::Nested(
+ "col", NANOARROW_TYPE_LIST,
+ {{"item", dictionary ? NANOARROW_TYPE_INT8 :
type}})}),
+ ADBC_STATUS_OK);
+ if (dictionary) {
+ auto* child = schema->children[0]->children[0];
+ ASSERT_EQ(ArrowSchemaAllocateDictionary(child), 0);
+ ASSERT_EQ(ArrowSchemaInitFromType(child->dictionary, type), 0);
+ }
+ ASSERT_EQ(ArrowArrayInitFromSchema(&array.value, &schema.value,
nullptr), 0);
+ ASSERT_EQ(ArrowArrayStartAppending(&array.value), 0);
+ auto* list = array->children[0];
+ auto* child = list->children[0];
+ ArrowBufferView value;
+ value.data.as_char = "x";
+ value.size_bytes = 1;
+ if (dictionary) {
+ ASSERT_EQ(ArrowArrayAppendBytes(child->dictionary, value), 0);
+ ASSERT_EQ(ArrowArrayAppendNull(child->dictionary, 1), 0);
+ ASSERT_EQ(ArrowArrayFinishBuildingDefault(child->dictionary, nullptr),
0);
+ }
+ for (int row = 0; row < 3; ++row) {
+ // Separate null indices from null dictionary values so each must set
+ // the header flag. A null index's default zero payload points to "x".
+ if (row == 0 || (!dictionary && row == 1)) {
+ ASSERT_EQ(ArrowArrayAppendNull(child, 1), 0);
+ } else if (dictionary) {
+ ASSERT_EQ(ArrowArrayAppendInt(child, row == 1 ? 1 : 0), 0);
+ } else {
+ ASSERT_EQ(ArrowArrayAppendBytes(child, value), 0);
+ }
+ if (dictionary) {
+ ASSERT_EQ(ArrowArrayAppendInt(child, 0), 0);
+ } else {
+ ASSERT_EQ(ArrowArrayAppendBytes(child, value), 0);
+ }
+ ASSERT_EQ(ArrowArrayFinishElement(list), 0);
+ ASSERT_EQ(ArrowArrayFinishElement(&array.value), 0);
+ }
+ ASSERT_EQ(ArrowArrayFinishBuildingDefault(&array.value, nullptr), 0);
+ PostgresCopyStreamWriteTester tester;
+ ASSERT_EQ(tester.Init(&schema.value, &array.value, *type_resolver_), 0);
+ ASSERT_EQ(tester.WriteAll(nullptr), ENODATA);
+ const auto buf = tester.WriteBuffer();
+ const std::vector<uint8_t> actual(buf.data, buf.data + buf.size_bytes);
+ if (dictionary) {
+ EXPECT_EQ(actual, reference);
+ } else {
+ reference = actual;
+ }
+ }
+ }
+}
+
TEST_F(PostgresCopyTest, PostgresCopyWriteMultiBatch) {
// Regression test for https://github.com/apache/arrow-adbc/issues/1310
adbc_validation::Handle<struct ArrowSchema> schema;
diff --git a/c/driver/postgresql/copy/writer.h
b/c/driver/postgresql/copy/writer.h
index a5f9ead1d5..040d55f77e 100644
--- a/c/driver/postgresql/copy/writer.h
+++ b/c/driver/postgresql/copy/writer.h
@@ -872,7 +872,7 @@ class PostgresCopyListFieldWriter : public
PostgresCopyFieldWriter {
}
constexpr int32_t ndim = 1;
- constexpr int32_t has_null_flags = 0;
+ int32_t has_null_flags = 0;
// TODO: the LARGE_LIST should use 64 bit indexes
int32_t start, end;
@@ -899,8 +899,21 @@ class PostgresCopyListFieldWriter : public
PostgresCopyFieldWriter {
// directly to our buffer
nanoarrow::UniqueBuffer tmp;
ArrowBufferInit(tmp.get());
+ const auto* child_view = array_view_->children[0];
for (auto i = start; i < end; ++i) {
- NANOARROW_RETURN_NOT_OK(child_->Write(tmp.get(), i, error));
+ bool is_null = ArrowArrayViewIsNull(child_view, i);
+ if (!is_null && child_view->dictionary != nullptr) {
+ const int64_t dict_index = ArrowArrayViewGetIntUnsafe(child_view, i);
+ is_null = ArrowArrayViewIsNull(child_view->dictionary, dict_index);
+ }
+ if (is_null) {
+ has_null_flags = 1;
+ constexpr int32_t field_size_bytes = -1;
+ NANOARROW_RETURN_NOT_OK(
+ WriteChecked<int32_t>(tmp.get(), field_size_bytes, error));
+ } else {
+ NANOARROW_RETURN_NOT_OK(child_->Write(tmp.get(), i, error));
+ }
}
const int32_t field_size_bytes = sizeof(ndim) + sizeof(has_null_flags) +
sizeof(child_oid_) + sizeof(dim) * ndim +
diff --git
a/c/driver/postgresql/validation/queries/ingest/fixed_size_list.txtcase
b/c/driver/postgresql/validation/queries/ingest/fixed_size_list.txtcase
new file mode 100644
index 0000000000..25d8325a6b
--- /dev/null
+++ b/c/driver/postgresql/validation/queries/ingest/fixed_size_list.txtcase
@@ -0,0 +1,201 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+// part: metadata
+
+[tags]
+sql-type-name = "ARRAY"
+field-type-name = ["_bool", "_int8", "_text", "_bool", "_int8", "_text"]
+
+// part: input_schema
+
+{
+ "format": "+s",
+ "children": [
+ {
+ "name": "idx",
+ "format": "l",
+ "flags": ["nullable"]
+ },
+ {
+ "name": "bools",
+ "format": "+w:3",
+ "flags": ["nullable"],
+ "children": [
+ {
+ "name": "item",
+ "format": "b",
+ "flags": ["nullable"]
+ }
+ ]
+ },
+ {
+ "name": "ints",
+ "format": "+w:3",
+ "flags": ["nullable"],
+ "children": [
+ {
+ "name": "item",
+ "format": "l",
+ "flags": ["nullable"]
+ }
+ ]
+ },
+ {
+ "name": "strings",
+ "format": "+w:3",
+ "flags": ["nullable"],
+ "children": [
+ {
+ "name": "item",
+ "format": "u",
+ "flags": ["nullable"]
+ }
+ ]
+ },
+ {
+ "name": "empty_bools",
+ "format": "+w:0",
+ "flags": ["nullable"],
+ "children": [
+ {
+ "name": "item",
+ "format": "b",
+ "flags": ["nullable"]
+ }
+ ]
+ },
+ {
+ "name": "empty_ints",
+ "format": "+w:0",
+ "flags": ["nullable"],
+ "children": [
+ {
+ "name": "item",
+ "format": "l",
+ "flags": ["nullable"]
+ }
+ ]
+ },
+ {
+ "name": "empty_strings",
+ "format": "+w:0",
+ "flags": ["nullable"],
+ "children": [
+ {
+ "name": "item",
+ "format": "u",
+ "flags": ["nullable"]
+ }
+ ]
+ }
+ ]
+}
+
+// part: input
+
+{"idx": 0, "bools": null, "ints": null, "strings": null, "empty_bools": null,
"empty_ints": null, "empty_strings": null}
+{"idx": 1, "bools": [true, false, true], "ints": [-9223372036854775808, 0,
9223372036854775807], "strings": ["", "hello", "NULL"], "empty_bools": [],
"empty_ints": [], "empty_strings": []}
+{"idx": 2, "bools": [true, null, false], "ints": [-1, null, 42], "strings":
["hello", null, ""], "empty_bools": [], "empty_ints": [], "empty_strings": []}
+{"idx": 3, "bools": [null, null, null], "ints": [null, null, null], "strings":
[null, null, null], "empty_bools": [], "empty_ints": [], "empty_strings": []}
+{"idx": 4, "bools": [false, true, false], "ints": [0, 1, -1], "strings": ["",
"NULL", "world"], "empty_bools": [], "empty_ints": [], "empty_strings": []}
+
+// PostgreSQL returns variable-size lists for array columns.
+// part: expected_schema
+
+{
+ "format": "+s",
+ "children": [
+ {
+ "name": "idx",
+ "format": "l",
+ "flags": ["nullable"]
+ },
+ {
+ "name": "bools",
+ "format": "+l",
+ "flags": ["nullable"],
+ "children": [
+ {
+ "name": "item",
+ "format": "b",
+ "flags": ["nullable"]
+ }
+ ]
+ },
+ {
+ "name": "ints",
+ "format": "+l",
+ "flags": ["nullable"],
+ "children": [
+ {
+ "name": "item",
+ "format": "l",
+ "flags": ["nullable"]
+ }
+ ]
+ },
+ {
+ "name": "strings",
+ "format": "+l",
+ "flags": ["nullable"],
+ "children": [
+ {
+ "name": "item",
+ "format": "u",
+ "flags": ["nullable"]
+ }
+ ]
+ },
+ {
+ "name": "empty_bools",
+ "format": "+l",
+ "flags": ["nullable"],
+ "children": [
+ {
+ "name": "item",
+ "format": "b",
+ "flags": ["nullable"]
+ }
+ ]
+ },
+ {
+ "name": "empty_ints",
+ "format": "+l",
+ "flags": ["nullable"],
+ "children": [
+ {
+ "name": "item",
+ "format": "l",
+ "flags": ["nullable"]
+ }
+ ]
+ },
+ {
+ "name": "empty_strings",
+ "format": "+l",
+ "flags": ["nullable"],
+ "children": [
+ {
+ "name": "item",
+ "format": "u",
+ "flags": ["nullable"]
+ }
+ ]
+ }
+ ]
+}
diff --git a/c/driver/postgresql/validation/queries/ingest/list.txtcase
b/c/driver/postgresql/validation/queries/ingest/list.txtcase
new file mode 100644
index 0000000000..40e83a659e
--- /dev/null
+++ b/c/driver/postgresql/validation/queries/ingest/list.txtcase
@@ -0,0 +1,80 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+// part: metadata
+
+[tags]
+sql-type-name = "ARRAY"
+field-type-name = ["_bool", "_int8", "_text"]
+
+// part: input_schema
+
+{
+ "format": "+s",
+ "children": [
+ {
+ "name": "idx",
+ "format": "l",
+ "flags": ["nullable"]
+ },
+ {
+ "name": "bools",
+ "format": "+l",
+ "flags": ["nullable"],
+ "children": [
+ {
+ "name": "item",
+ "format": "b",
+ "flags": ["nullable"]
+ }
+ ]
+ },
+ {
+ "name": "ints",
+ "format": "+l",
+ "flags": ["nullable"],
+ "children": [
+ {
+ "name": "item",
+ "format": "l",
+ "flags": ["nullable"]
+ }
+ ]
+ },
+ {
+ "name": "strings",
+ "format": "+l",
+ "flags": ["nullable"],
+ "children": [
+ {
+ "name": "item",
+ "format": "u",
+ "flags": ["nullable"]
+ }
+ ]
+ }
+ ]
+}
+
+// part: input
+
+{"idx": 0, "bools": null, "ints": null, "strings": null}
+{"idx": 1, "bools": [], "ints": [], "strings": []}
+{"idx": 2, "bools": [true, false, true], "ints": [-9223372036854775808, 0,
9223372036854775807], "strings": ["", "hello", "NULL"]}
+{"idx": 3, "bools": [true, null, false], "ints": [-1, null, 42], "strings":
["hello", null, ""]}
+{"idx": 4, "bools": [null, null, null], "ints": [null, null, null], "strings":
[null, null, null]}
+{"idx": 5, "bools": [false, true, false], "ints": [0, 1, -1], "strings": ["",
"NULL", "world"]}
diff --git a/compose.yaml b/compose.yaml
index 4a56ea449b..90ffbb5265 100644
--- a/compose.yaml
+++ b/compose.yaml
@@ -357,7 +357,7 @@ services:
cedardb-test:
container_name: adbc_cedardb_test
- image: cedardb/cedardb:v2026-08-13
+ image: cedardb/cedardb:v2026-09-03
environment:
CEDAR_PASSWORD: CedarDB2026!
healthcheck:
