paleolimbot commented on code in PR #2919:
URL: https://github.com/apache/arrow-adbc/pull/2919#discussion_r2127873308
##########
c/driver/postgresql/copy/postgres_copy_reader_test.cc:
##########
@@ -797,6 +797,88 @@ TEST(PostgresCopyUtilsTest, PostgresCopyReadArray) {
ASSERT_EQ(data_buffer[4], 123);
}
+TEST(PostgresCopyUtilsTest, PostgresCopyReadInt2vector) {
+ ArrowBufferView data;
+ data.data.as_uint8 = kTestPgCopyInt2vector;
+ data.size_bytes = sizeof(kTestPgCopyInt2vector);
+
+ // We don't actually use int2vector for the reader; we treat it as
+ // equivalent to int2 ARRAY.
+ auto col_type = PostgresType(PostgresTypeId::kInt2).Array();
+ PostgresType input_type(PostgresTypeId::kRecord);
Review Comment:
I think you probably do want the type id to be int2vector (instantiating the
copy reader is an implementation detail I don't think needs to live outside
that one function?)
##########
c/driver/postgresql/postgres_type.h:
##########
@@ -475,6 +480,15 @@ class PostgresTypeResolver {
break;
}
+ case PostgresTypeId::kInt2vector: {
+ PostgresType child;
+ NANOARROW_RETURN_NOT_OK(Find(GetOID(PostgresTypeId::kInt2), &child,
error));
+ mapping_.insert({item.oid, child.Array(item.oid, item.typname)});
+ reverse_mapping_.insert({static_cast<int32_t>(base.type_id()),
item.oid});
+ array_mapping_.insert({child.oid(), item.oid});
+ break;
+ }
Review Comment:
This seems like it might interfere with or overwrite the mapping for the
array of int2, which is possibly a distinct type in the table?
##########
c/driver/postgresql/postgres_type_test.cc:
##########
@@ -441,4 +441,20 @@ TEST(PostgresTypeTest, PostgresTypeResolveRecord) {
EXPECT_EQ(type.child(1).type_id(), PostgresTypeId::kText);
}
+TEST(PostgresTypeTest, PostgresTypeResolveInt2vector) {
+ MockTypeResolver resolver;
+ ASSERT_EQ(resolver.Init(), NANOARROW_OK);
+
+ PostgresType type;
+
+ const auto int2vector_oid = resolver.GetOID(PostgresTypeId::kInt2vector);
+ const auto int2_oid = resolver.GetOID(PostgresTypeId::kInt2);
+ EXPECT_EQ(resolver.Find(int2vector_oid, &type, nullptr), NANOARROW_OK);
+ EXPECT_EQ(type.oid(), int2vector_oid);
+ EXPECT_EQ(type.typname(), "int2vector");
+ EXPECT_EQ(type.type_id(), PostgresTypeId::kArray);
+ EXPECT_EQ(type.child(0).oid(), int2_oid);
+ EXPECT_EQ(type.child(0).type_id(), PostgresTypeId::kInt2);
Review Comment:
Is there a reason that `type.type_id()` can't just be
`PostgresTypeId::kInt2Vector`?
--
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]
