feat(c/driver/postgresql): add read support for int2vector #2919
Conversation
paleolimbot
left a comment
paleolimbot
left a comment
There was a problem hiding this comment.
Thank you!
It is worth a quick SQL check to make sure this actually can be piped all the way though (and that the SQL query that finds all the type OIDs finds the row for int2vector).
c/driver/postgresql/postgres_type.h
Outdated
| 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; | ||
| } |
There was a problem hiding this 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?
There was a problem hiding this comment.
item.oid is still the int2vector's OID here. I'm just grabbing the int2 OID so I can use it as the child.
Alternatively, I could ignore this, and in the int2vector special case above, I could just directly allocate a child schema (that probably makes more sense)
| 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); |
There was a problem hiding this comment.
Is there a reason that type.type_id() can't just be PostgresTypeId::kInt2Vector?
There was a problem hiding this comment.
Did you mean to put this on the line up above? But it's because I'm faking int2vector as effectively alias for array of int2.
| // 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); |
There was a problem hiding this 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?)
|
I did manually test with Python that the query actually works but I'll add and commit a test in C++ |
|
Updated to push the special-case of int2vector around a bit |
2 participants
Fixes #2903.