Skip to content

Commit dc14156

Browse files
committed
sql: ensure comments are looked up for create_type_statements
Previously, crdb_internal.create_type_statements did not look up comment metadata for point lookup indexes. This could lead to missing comments or an assertion firing on test builds. To address this, this patch adds a new WithMetaData flag in the collections which can be combined for leased descriptor scenarios like this. This allows a point lookup of comments to be used when using leased descriptors for catalog views. Corrected Version Fixes: #157916 Release note: None
1 parent e837db7 commit dc14156

File tree

6 files changed

+25
-6
lines changed

6 files changed

+25
-6
lines changed

pkg/sql/catalog/descs/getters.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -530,6 +530,14 @@ func (b ByIDGetterBuilder) WithoutSynthetic() ByIDGetterBuilder {
530530
return b
531531
}
532532

533+
// WithMetaData configures the byIDGetterBuilder to read zone configs and comments
534+
// for lease descriptors. This is always acquired for descriptors from storage,
535+
// and may need an extra round trip.
536+
func (b ByIDGetterBuilder) WithMetaData() ByIDGetterBuilder {
537+
b.flags.layerFilters.withMetadata = true
538+
return b
539+
}
540+
533541
// WithoutDropped configures the ByIDGetterBuilder to error on descriptors
534542
// which are in a dropped state.
535543
func (b ByIDGetterBuilder) WithoutDropped() ByIDGetterBuilder {

pkg/sql/crdb_internal.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3227,7 +3227,7 @@ CREATE TABLE crdb_internal.create_type_statements (
32273227
addRow func(...tree.Datum) error,
32283228
) (matched bool, err error) {
32293229
id := descpb.ID(tree.MustBeDInt(unwrappedConstraint))
3230-
scName, typDesc, err := getSchemaAndTypeByTypeID(ctx, p, id)
3230+
scName, typDesc, err := getSchemaAndTypeByTypeID(ctx, p, id, true /* includeMetaData */)
32313231
if err != nil || typDesc == nil {
32323232
return false, err
32333233
}

pkg/sql/logictest/testdata/logic_test/crdb_internal

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# 3node-tenant is blocked from running this file due to heavy reliance on
22
# unavailable node IDs in this test.
3-
# LogicTest: !3node-tenant-default-configs !local-prepared
3+
# LogicTest: !3node-tenant-default-configs !local-prepared local-leased-descriptors default-configs
44

55
query error database "crdb_internal" does not exist
66
ALTER DATABASE crdb_internal RENAME TO not_crdb_internal

pkg/sql/logictest/tests/local-leased-descriptors/BUILD.bazel

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ go_test(
1212
"//build/toolchains:is_heavy": {"test.Pool": "heavy"},
1313
"//conditions:default": {"test.Pool": "large"},
1414
}),
15-
shard_count = 11,
15+
shard_count = 12,
1616
tags = ["cpu:1"],
1717
deps = [
1818
"//pkg/base",

pkg/sql/logictest/tests/local-leased-descriptors/generated_test.go

Lines changed: 7 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/sql/pg_catalog.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3796,9 +3796,13 @@ func addPGAttributeRowForCompositeType(
37963796
}
37973797

37983798
func getSchemaAndTypeByTypeID(
3799-
ctx context.Context, p *planner, id descpb.ID,
3799+
ctx context.Context, p *planner, id descpb.ID, includeMetaData bool,
38003800
) (catalog.SchemaDescriptor, catalog.TypeDescriptor, error) {
3801-
typDesc, err := descs.GetCatalogDescriptorGetter(p.Descriptors(), p.txn, &p.EvalContext().Settings.SV).WithoutNonPublic().Get().Type(ctx, id)
3801+
getter := descs.GetCatalogDescriptorGetter(p.Descriptors(), p.txn, &p.EvalContext().Settings.SV)
3802+
if includeMetaData {
3803+
getter = getter.WithMetaData()
3804+
}
3805+
typDesc, err := getter.WithoutNonPublic().Get().Type(ctx, id)
38023806
if err != nil {
38033807
// If the type was not found, it may be a table.
38043808
if !(errors.Is(err, catalog.ErrDescriptorNotFound) || pgerror.GetPGCode(err) == pgcode.UndefinedObject) {
@@ -3897,7 +3901,7 @@ https://www.postgresql.org/docs/9.5/catalog-pg-type.html`,
38973901

38983902
id := typedesc.UserDefinedTypeOIDToID(ooid)
38993903

3900-
sc, typDesc, err := getSchemaAndTypeByTypeID(ctx, p, id)
3904+
sc, typDesc, err := getSchemaAndTypeByTypeID(ctx, p, id, false /* includeMetaData */)
39013905
if err != nil || typDesc == nil {
39023906
return false, err
39033907
}

0 commit comments

Comments
 (0)