sql: create implicit array types for UDTs #49678
Conversation
|
This change is |
|
cc @ajwerner, @lucy-zhang if you guys want to take a look. |
pkg/sql/types/types.proto
Outdated
| @@ -513,4 +513,10 @@ message InternalType { | |||
| // StableTypeID is set only for User Defined Types. | |||
| optional uint32 stable_type_id = 15 | |||
There was a problem hiding this comment.
wondering if it's worth keeping this in a struct, e.g. optional UserDefinedTypeMetadata udt_metadata... and make this field and the one below non nullable. then it's easy to tell whether it's a UDT or not (udt_metadata != nil)
There was a problem hiding this comment.
Oh, that might be a good idea! IDK if we are worried about extra allocations when reading types protos though (maybe thats nothing to worry about).
There was a problem hiding this comment.
I'll wait until more fields pop up -- i think 3 is a good tipping point for its own struct.
There was a problem hiding this comment.
i think 2 is :3 (it's especially good forward planning for protobufs)
rohany
force-pushed
the
array-types
branch
3 times, most recently
from
ffb96b6
to
1a83c1d
Compare
| INSERT INTO enum_array VALUES (ARRAY['hello'], ARRAY['hello']), (ARRAY['howdy'], ARRAY['howdy']) | ||
|
|
||
| query TT rowsort | ||
| SELECT * FROM enum_array |
There was a problem hiding this comment.
Can you check that the elements of this column are properly enum-typed?
There was a problem hiding this comment.
what do you mean by this?
There was a problem hiding this comment.
Like - select pg_typeof(x[0]) from enum_array should return greeting, and x[0] should be a greeting and not a string. Not sure how you can test that. I'm sure the type system mostly guarantees this already. I think it'd be good to have a smoke test though that you can roundtrip enums through arrays, through disk, and back.
There was a problem hiding this comment.
Done, dang you caught a sneaky bug with names not getting set up in the right spots.
rohany
force-pushed
the
array-types
branch
3 times, most recently
from
17f6a35
to
5a9241a
Compare
pkg/sql/types/types.proto
Outdated
| optional uint32 stable_type_id = 15 | ||
| [(gogoproto.nullable) = false, (gogoproto.customname) = "StableTypeID"]; | ||
| // UDTMetadata is populated for user defined types. | ||
| optional SerializedUserDefinedTypeMetadata udt_metadata = 15 [(gogoproto.customname) = "UDTMetadata"]; |
There was a problem hiding this comment.
This is safe to change only because stable_type_id was added in in this release.
pkg/sql/types/types.proto
Outdated
| // SerializedUserDefinedTypeMetadata contains user defined type metadata | ||
| // that will be serialized to disk, unlike other user defined type metadata | ||
| // that is only stored in memory once a type is resolved. | ||
| message SerializedUserDefinedTypeMetadata { |
There was a problem hiding this comment.
minor nit: doesn't need Serialized as a prefix, as protobufs imply serialization.
(ok if you decide to keep it, in case there's another UDT struct somewhere, but on first glance looks unnecessary)
There was a problem hiding this comment.
We already have a UserDefinedTypeMetadata for the in memory metadata, so I need some way to differentiate -- do you have a different suggestion?
There was a problem hiding this comment.
Ok i renamed to PersistentUserDefinedTypeMetadata -- thoughts?
There was a problem hiding this comment.
ah fair enough. works for me.
Fixes cockroachdb#49197. This PR adds the Postgres behavior of creating an implicit array type for a user defined type when it is created. The implicit array type will track the user defined type. Release note: None
|
TFTRs! bors r=otan,jordanlewis |
Build succeeded |
Fixes #49197.
This PR adds the Postgres behavior of creating an implicit array type
for a user defined type when it is created. The implicit array type will
track the user defined type.
Release note: None