descs: push descriptor type hydration into the desc.Collection #51621
Conversation
|
This change is |
There was a problem hiding this comment.
Reviewed 3 of 4 files at r1.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @ajwerner, @lucy-zhang, and @rohany)
pkg/sql/catalog/descs/collection.go, line 263 at r1 (raw file):
// not shared. return desc, sqlbase.HydrateTypesInTableDescriptor(ctx, t.TableDesc(), sqlbase.TypeLookupFunc(getType)) case *sqlbase.ImmutableTableDescriptor:
How does one get a handle to an ImmutableTableDescriptor that isn't hydrated?
The lease manager/uncached accessor is pulling up table descs and creating immutable descs to hand out to higher levels of name resolution (i.e. the collection) -- those aren't coming hydrated. |
There was a problem hiding this comment.
The lease manager/uncached accessor is pulling up table descs and creating immutable descs to hand out to higher levels of name resolution (i.e. the collection) -- those aren't coming hydrated.
That's sort of exactly my question. Is the Collection the only thing that ever has handles to unhydrated ImmutableTableDescriptors?
Reviewable status:
|
Yeah, I think that is a reasonable separation -- everything below the collection needs to hydrate descriptors if they want to use them and know that they have user defined types. |
rohany
force-pushed
the
hydrated-type-cache
branch
from
91a0c1b to
82a59a2
Compare
|
Rebased, ready for review. |
There was a problem hiding this comment.
Reviewed 4 of 4 files at r2.
Reviewable status:
Fixes cockroachdb#49484. Preparation for cockroachdb#51385. Up until now, the planner was responsible for installing user defined type metadata in tables that contained user defined types. This was slightly messy and caused leakage of responsibility regarding when descriptors had user defined types vs when they didn't. This commit pushes that responsibility into the descs.Collection. It also paves the way for work to avoid copying ImmutableTableDescriptors that contain user defined types every time that they need hydration. Release note: None
rohany
force-pushed
the
hydrated-type-cache
branch
from
82a59a2 to
eaeb7c1
Compare
|
bors r+ |
|
Build succeeded: |
Fixes #49484.
Preparation for #51385.
Up until now, the
plannerwas responsible for installing user definedtype metadata in tables that contained user defined types. This was
slightly messy and caused leakage of responsibility regarding when
descriptors had user defined types vs when they didn't. This commit
pushes that responsibility into the
descs.Collection. It also pavesthe way for work to avoid copying
ImmutableTableDescriptors thatcontain user defined types every time that they need hydration.
Release note: None