lidavidm commented on code in PR #4320:
URL: https://github.com/apache/arrow-adbc/pull/4320#discussion_r3331037220
##########
c/driver/postgresql/postgresql_test.cc:
##########
@@ -1892,6 +1892,103 @@ TEST_F(PostgresStatementTest,
EmptyStringAndBinaryParameter) {
ASSERT_THAT(AdbcStatementRelease(&statement, &error), IsOkStatus(&error));
}
+// Regression test for https://github.com/apache/arrow-adbc/issues/4319.
+// The parameterized prepared-statement path (BindAndExecuteCurrentRow)
+// constructs its field writers via the same MakeCopyFieldWriter factory
+// as the COPY ingest path, so the offset bug in PostgresCopyListFieldWriter
+// drifts list-typed bound parameters the same way. This exercises the bind
+// path end-to-end with an INSERT … ON CONFLICT DO UPDATE upsert whose list
+// parameter comes from a sliced Arrow array (parent.offset > 0).
+TEST_F(PostgresStatementTest, BindUpsertWithSlicedListParameter) {
+ ASSERT_THAT(quirks()->DropTable(&connection, "adbc_test", &error),
IsOkStatus(&error));
+ ASSERT_THAT(AdbcStatementNew(&connection, &statement, &error),
IsOkStatus(&error));
+
+ ASSERT_THAT(
+ AdbcStatementSetSqlQuery(
+ &statement, "CREATE TABLE adbc_test (id INT PRIMARY KEY, tags TEXT[]
NOT NULL)",
+ &error),
+ IsOkStatus(&error));
+ {
+ adbc_validation::StreamReader reader;
+ ASSERT_THAT(
+ AdbcStatementExecuteQuery(&statement, &reader.stream.value, nullptr,
&error),
+ IsOkStatus(&error));
+ }
+
+ // 6-row struct(int32, list<string>) where odd rows have 1-element tags
+ // and even rows have 2-element tags. The size difference makes drift
+ // structurally observable, not just value-different.
+ adbc_validation::Handle<struct ArrowSchema> bind_schema;
+ adbc_validation::Handle<struct ArrowArray> bind;
+ struct ArrowError na_error;
+ ASSERT_EQ(adbc_validation::MakeSchema(
+ &bind_schema.value,
+ {{"id", NANOARROW_TYPE_INT32},
+ adbc_validation::SchemaField::Nested(
+ "tags", NANOARROW_TYPE_LIST, {{"item",
NANOARROW_TYPE_STRING}})}),
+ ADBC_STATUS_OK);
+
+ ASSERT_EQ(
+ (adbc_validation::MakeBatch<int32_t, std::vector<std::string>>(
+ &bind_schema.value, &bind.value, &na_error, {0, 1, 2, 3, 4, 5},
+ {std::vector<std::string>{"r0a", "r0b"},
std::vector<std::string>{"r1x"},
+ std::vector<std::string>{"r2a", "r2b"},
std::vector<std::string>{"r3x"},
+ std::vector<std::string>{"r4a", "r4b"},
std::vector<std::string>{"r5x"}})),
+ ADBC_STATUS_OK);
+
+ // Hide rows 0..2 by setting offset/length on the struct root and on
+ // each child column (nanoarrow does not propagate the struct's offset).
Review Comment:
```suggestion
// Hide rows 0..2 by setting offset/length on the struct root and on
// each child column
```
##########
c/driver/postgresql/copy/postgres_copy_writer_test.cc:
##########
@@ -1587,6 +1587,121 @@ TEST_F(PostgresCopyTest,
PostgresCopyWriteFixedSizeListInteger) {
}
}
+// Regression test for https://github.com/apache/arrow-adbc/issues/4319.
+// When the source array has offset > 0 (a sliced parent), the list writer
+// must read child offsets at (array_view->offset + index), not at index.
+// Writing rows 3..5 of a 6-row source via offset/length must produce the
+// same body as writing those rows as a fresh 3-row array.
+TEST_P(PostgresCopyListTest, PostgresCopyWriteListSlicedMatchesDirect) {
+ adbc_validation::Handle<struct ArrowSchema> schema;
+ adbc_validation::Handle<struct ArrowArray> source;
+ adbc_validation::Handle<struct ArrowArray> tail;
+ struct ArrowError na_error;
+
+ ASSERT_EQ(adbc_validation::MakeSchema(
+ &schema.value, {adbc_validation::SchemaField::Nested(
+ "col", GetParam(), {{"item",
NANOARROW_TYPE_INT32}})}),
+ ADBC_STATUS_OK);
+
+ ASSERT_EQ(
+ adbc_validation::MakeBatch<std::vector<int32_t>>(
+ &schema.value, &source.value, &na_error,
+ {std::vector<int32_t>{1, 2}, std::vector<int32_t>{3, 4, 5},
std::nullopt,
+ std::vector<int32_t>{6}, std::vector<int32_t>{7, 8},
std::vector<int32_t>{9}}),
+ ADBC_STATUS_OK);
+
+ ASSERT_EQ(
+ adbc_validation::MakeBatch<std::vector<int32_t>>(
+ &schema.value, &tail.value, &na_error,
+ {std::vector<int32_t>{6}, std::vector<int32_t>{7, 8},
std::vector<int32_t>{9}}),
+ ADBC_STATUS_OK);
+
+ PostgresCopyStreamWriteTester ref_tester;
+ ASSERT_EQ(ref_tester.Init(&schema.value, &tail.value, *type_resolver_),
NANOARROW_OK);
+ ASSERT_EQ(ref_tester.WriteAll(nullptr), ENODATA);
+ const struct ArrowBuffer ref_buf = ref_tester.WriteBuffer();
+
+ // Slice: hide the first 3 rows by setting offset/length on the struct
+ // root and on the list-typed column. nanoarrow does not propagate the
+ // struct's offset down to children, so both levels need updating.
Review Comment:
```suggestion
// Slice: hide the first 3 rows by setting offset/length on the struct
// root and on the list-typed column.
```
(offsets aren't meant to be propagated; an offset on the child would be
combined with the offset on the struct)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
