From f62d2aaf07bbd09a7cb81156b2ac57d846fe796c Mon Sep 17 00:00:00 2001 From: Marius Posta Date: Wed, 14 Dec 2022 22:20:56 -0500 Subject: [PATCH] descs: fix GetAllFromStorageUnvalidated method This commit fixes a bug in which persisted catalog changes were not visible in the results of this descs.Collection's method when the transation already involved a full catalog scan before those changes. Release note: None --- pkg/sql/catalog/descs/collection.go | 10 +++++----- pkg/sql/catalog/internal/catkv/catalog_reader.go | 5 ++--- pkg/sql/catalog/internal/catkv/catalog_reader_test.go | 2 +- 3 files changed, 8 insertions(+), 9 deletions(-) diff --git a/pkg/sql/catalog/descs/collection.go b/pkg/sql/catalog/descs/collection.go index a8adeb9d7c11..b762b7289393 100644 --- a/pkg/sql/catalog/descs/collection.go +++ b/pkg/sql/catalog/descs/collection.go @@ -710,14 +710,14 @@ func (tc *Collection) GetAll(ctx context.Context, txn *kv.Txn) (nstree.Catalog, return ret.Catalog, nil } -// GetAllFromStorageUnvalidated delegates to the catkv.CatalogReader's ScanAll. -// Nothing is validated or hydrated. This is to be used sparingly and only in -// situations which warrant it, where an unmediated view of the stored catalog -// is explicitly desired for some purpose of observability. +// GetAllFromStorageUnvalidated delegates to an uncached catkv.CatalogReader's +// ScanAll method. Nothing is cached, validated or hydrated. This is to be used +// sparingly and only in situations which warrant it, where an unmediated view +// of the stored catalog is explicitly desired for observability. func (tc *Collection) GetAllFromStorageUnvalidated( ctx context.Context, txn *kv.Txn, ) (nstree.Catalog, error) { - return tc.cr.ScanAll(ctx, txn) + return catkv.NewUncachedCatalogReader(tc.codec()).ScanAll(ctx, txn) } // GetAllDatabases is like GetAll but filtered to non-dropped databases. diff --git a/pkg/sql/catalog/internal/catkv/catalog_reader.go b/pkg/sql/catalog/internal/catkv/catalog_reader.go index 22130fc5d000..461129307890 100644 --- a/pkg/sql/catalog/internal/catkv/catalog_reader.go +++ b/pkg/sql/catalog/internal/catkv/catalog_reader.go @@ -99,10 +99,9 @@ type CatalogReader interface { ) (nstree.Catalog, error) } -// NewTestingUncachedCatalogReader is the constructor for the default +// NewUncachedCatalogReader is the constructor for the default // CatalogReader implementation without a SystemDatabaseCache. -// This should not be used outside of testing purposes. -func NewTestingUncachedCatalogReader(codec keys.SQLCodec) CatalogReader { +func NewUncachedCatalogReader(codec keys.SQLCodec) CatalogReader { return &catalogReader{ codec: codec, } diff --git a/pkg/sql/catalog/internal/catkv/catalog_reader_test.go b/pkg/sql/catalog/internal/catkv/catalog_reader_test.go index 9c6b40a659f0..8fb11316d2fa 100644 --- a/pkg/sql/catalog/internal/catkv/catalog_reader_test.go +++ b/pkg/sql/catalog/internal/catkv/catalog_reader_test.go @@ -64,7 +64,7 @@ func TestDataDriven(t *testing.T) { v := execCfg.Settings.Version.ActiveVersion(ctx) sdc := catkv.NewSystemDatabaseCache(execCfg.Codec, execCfg.Settings) ccr := catkv.NewCatalogReader(execCfg.Codec, v, sdc, nil /* maybeMonitor */) - ucr := catkv.NewTestingUncachedCatalogReader(execCfg.Codec) + ucr := catkv.NewUncachedCatalogReader(execCfg.Codec) datadriven.RunTest(t, path, func(t *testing.T, d *datadriven.TestData) (ret string) { h := testHelper{