diff --git a/pkg/sql/backfill.go b/pkg/sql/backfill.go index 3ae80f31c355..7921a4e5a41d 100644 --- a/pkg/sql/backfill.go +++ b/pkg/sql/backfill.go @@ -28,7 +28,6 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/catalog" "github.com/cockroachdb/cockroach/pkg/sql/catalog/descs" "github.com/cockroachdb/cockroach/pkg/sql/catalog/lease" - "github.com/cockroachdb/cockroach/pkg/sql/catalog/resolver" "github.com/cockroachdb/cockroach/pkg/sql/execinfrapb" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror" @@ -655,21 +654,6 @@ func (sc *SchemaChanger) truncateIndexes( if err != nil { return err } - - // Hydrate types used in the retrieved table. - // TODO (rohany): This can be removed once table access from the - // desc.Collection returns tables with hydrated types. - typLookup := func(ctx context.Context, id sqlbase.ID) (*tree.TypeName, sqlbase.TypeDescriptorInterface, error) { - return resolver.ResolveTypeDescByID(ctx, txn, sc.execCfg.Codec, id, tree.ObjectLookupFlags{}) - } - if err := sqlbase.HydrateTypesInTableDescriptor( - ctx, - tableDesc.TableDesc(), - sqlbase.TypeLookupFunc(typLookup), - ); err != nil { - return err - } - rd, err := row.MakeDeleter( ctx, txn, diff --git a/pkg/sql/catalog/descs/collection.go b/pkg/sql/catalog/descs/collection.go index 4d1bc55abcb2..7da8859089b5 100644 --- a/pkg/sql/catalog/descs/collection.go +++ b/pkg/sql/catalog/descs/collection.go @@ -29,6 +29,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/sqlbase" "github.com/cockroachdb/cockroach/pkg/util/log" + "github.com/cockroachdb/cockroach/pkg/util/protoutil" "github.com/cockroachdb/errors" ) @@ -162,7 +163,11 @@ func (tc *Collection) GetMutableTableDescriptor( if !ok { return nil, nil } - return mutDesc, nil + hydrated, err := tc.hydrateTypesInTableDesc(ctx, txn, mutDesc) + if err != nil { + return nil, err + } + return hydrated.(*sqlbase.MutableTableDescriptor), nil } func (tc *Collection) getMutableObjectDescriptor( @@ -288,7 +293,11 @@ func (tc *Collection) GetTableVersion( } return nil, nil } - return table, nil + hydrated, err := tc.hydrateTypesInTableDesc(ctx, txn, table) + if err != nil { + return nil, err + } + return hydrated.(*sqlbase.ImmutableTableDescriptor), nil } func (tc *Collection) getObjectVersion( @@ -434,7 +443,11 @@ func (tc *Collection) GetTableVersionByID( return nil, sqlbase.NewUndefinedRelationError( &tree.TableRef{TableID: int64(tableID)}) } - return table, nil + hydrated, err := tc.hydrateTypesInTableDesc(ctx, txn, table) + if err != nil { + return nil, err + } + return hydrated.(*sqlbase.ImmutableTableDescriptor), nil } func (tc *Collection) getDescriptorVersionByID( @@ -506,7 +519,12 @@ func (tc *Collection) GetMutableTableVersionByID( if err != nil { return nil, err } - return desc.(*sqlbase.MutableTableDescriptor), nil + table := desc.(*sqlbase.MutableTableDescriptor) + hydrated, err := tc.hydrateTypesInTableDesc(ctx, txn, table) + if err != nil { + return nil, err + } + return hydrated.(*sqlbase.MutableTableDescriptor), nil } func (tc *Collection) getMutableDescriptorByID( @@ -528,6 +546,46 @@ func (tc *Collection) getMutableDescriptorByID( return desc, nil } +// hydrateTypesInTableDesc installs user defined type metadata in all types.T +// present in the input TableDescriptor. It always returns the same type of +// TableDescriptor that was passed in. It ensures that ImmutableTableDescriptors +// are not modified during the process of metadata installation. +func (tc *Collection) hydrateTypesInTableDesc( + ctx context.Context, txn *kv.Txn, desc sqlbase.TableDescriptorInterface, +) (sqlbase.TableDescriptorInterface, error) { + getType := func(ctx context.Context, id sqlbase.ID) (*tree.TypeName, sqlbase.TypeDescriptorInterface, error) { + // TODO (rohany): Use the collection API's here. + return resolver.ResolveTypeDescByID(ctx, txn, tc.codec(), id, tree.ObjectLookupFlags{}) + } + switch t := desc.(type) { + case *sqlbase.MutableTableDescriptor: + // It is safe to hydrate directly into MutableTableDescriptor since it is + // not shared. + return desc, sqlbase.HydrateTypesInTableDescriptor(ctx, t.TableDesc(), sqlbase.TypeLookupFunc(getType)) + case *sqlbase.ImmutableTableDescriptor: + // ImmutableTableDescriptors need to be copied before hydration, because + // they are potentially read by multiple threads. If there aren't any user + // defined types in the descriptor, then return early. + if !t.ContainsUserDefinedTypes() { + return desc, nil + } + + // TODO (rohany, ajwerner): Here we would look into the cached set of + // hydrated table descriptors and potentially return without having to + // make a copy. However, we could avoid hitting the cache if any of the + // user defined types have been modified in this transaction. + + // Make a copy of the underlying descriptor before hydration. + descBase := protoutil.Clone(t.TableDesc()).(*sqlbase.TableDescriptor) + if err := sqlbase.HydrateTypesInTableDescriptor(ctx, descBase, sqlbase.TypeLookupFunc(getType)); err != nil { + return nil, err + } + return sqlbase.NewImmutableTableDescriptor(*descBase), nil + default: + return desc, nil + } +} + // ReleaseSpecifiedLeases releases the leases for the descriptors with ids in // the passed slice. Errors are logged but ignored. func (tc *Collection) ReleaseSpecifiedLeases(ctx context.Context, descs []lease.IDVersion) { diff --git a/pkg/sql/opt/exec/execbuilder/testdata/explain b/pkg/sql/opt/exec/execbuilder/testdata/explain index 456c41716c98..558484b68d9d 100644 --- a/pkg/sql/opt/exec/execbuilder/testdata/explain +++ b/pkg/sql/opt/exec/execbuilder/testdata/explain @@ -601,16 +601,16 @@ CREATE TABLE tc (a INT, b INT, INDEX c(a), FAMILY "primary" (a, b, rowid)) query TTTTT EXPLAIN (VERBOSE) SELECT * FROM tc WHERE a = 10 ORDER BY b ---- -· distribution local · · -· vectorized true · · -sort · · (a, b) +b - │ order +b · · - └── index-join · · (a, b) · - │ table tc@primary · · - │ key columns rowid · · - └── scan · · (a, rowid[hidden]) · -· table tc@c · · -· spans /10-/11 · · +· distribution local · · +· vectorized true · · +sort · · (a, b) +b + │ order +b · · + └── index-join · · (a, b) · + │ table tc@primary · · + │ key columns rowid · · + └── scan · · (a, rowid) · +· table tc@c · · +· spans /10-/11 · · query TTTTT colnames EXPLAIN (TYPES) INSERT INTO t VALUES (1, 2) diff --git a/pkg/sql/opt/exec/execbuilder/testdata/insert b/pkg/sql/opt/exec/execbuilder/testdata/insert index 071ef7c1b116..1750184734fc 100644 --- a/pkg/sql/opt/exec/execbuilder/testdata/insert +++ b/pkg/sql/opt/exec/execbuilder/testdata/insert @@ -428,8 +428,8 @@ INSERT INTO insert_t (SELECT length(k), 2 FROM kv ORDER BY k || v LIMIT 10) RETU · vectorized false · · render · · ("?column?") · │ render 0 x + v · · - └── run · · (x, v, rowid[hidden]) · - └── insert · · (x, v, rowid[hidden]) · + └── run · · (x, v, rowid) · + └── insert · · (x, v, rowid) · │ into insert_t(x, v, rowid) · · │ strategy inserter · · └── render · · (length, "?column?", column11) · @@ -462,18 +462,18 @@ BEGIN; ALTER TABLE mutation DROP COLUMN y query TTTTT EXPLAIN (VERBOSE) INSERT INTO mutation(x) VALUES (2) RETURNING * ---- -· distribution local · · -· vectorized false · · -render · · (x) · - │ render 0 x · · - └── run · · (x, rowid[hidden]) · - └── insert-fast-path · · (x, rowid[hidden]) · -· into mutation(x, rowid, y) · · -· strategy inserter · · -· size 3 columns, 1 row · · -· row 0, expr 0 2 · · -· row 0, expr 1 unique_rowid() · · -· row 0, expr 2 10 · · +· distribution local · · +· vectorized false · · +render · · (x) · + │ render 0 x · · + └── run · · (x, rowid) · + └── insert-fast-path · · (x, rowid) · +· into mutation(x, rowid, y) · · +· strategy inserter · · +· size 3 columns, 1 row · · +· row 0, expr 0 2 · · +· row 0, expr 1 unique_rowid() · · +· row 0, expr 2 10 · · statement ok ROLLBACK @@ -527,8 +527,8 @@ root · · └── spool · · (z) · └── render · · (z) · │ render 0 z · · - └── run · · (z, rowid[hidden]) · - └── insert · · (z, rowid[hidden]) · + └── run · · (z, rowid) · + └── insert · · (z, rowid) · │ into xyz(x, y, z, rowid) · · │ strategy inserter · · └── render · · (a, b, c, column11) · diff --git a/pkg/sql/opt/exec/execbuilder/testdata/orderby b/pkg/sql/opt/exec/execbuilder/testdata/orderby index c3ff806c8e5c..c743d73feb80 100644 --- a/pkg/sql/opt/exec/execbuilder/testdata/orderby +++ b/pkg/sql/opt/exec/execbuilder/testdata/orderby @@ -907,15 +907,15 @@ CREATE TABLE xyz (x INT, y INT, z INT, INDEX(z,y)) query TTTTT EXPLAIN (VERBOSE) SELECT * FROM xyz WHERE z=1 AND x=y ORDER BY x; ---- -· distribution local · · -· vectorized true · · -sort · · (x, y, z) +x - │ order +x · · - └── filter · · (x, y, z) · - │ filter x = y · · - └── index-join · · (x, y, z) · - │ table xyz@primary · · - │ key columns rowid · · - └── scan · · (y, z, rowid[hidden]) · -· table xyz@xyz_z_y_idx · · -· spans /1/!NULL-/2 · · +· distribution local · · +· vectorized true · · +sort · · (x, y, z) +x + │ order +x · · + └── filter · · (x, y, z) · + │ filter x = y · · + └── index-join · · (x, y, z) · + │ table xyz@primary · · + │ key columns rowid · · + └── scan · · (y, z, rowid) · +· table xyz@xyz_z_y_idx · · +· spans /1/!NULL-/2 · · diff --git a/pkg/sql/opt/exec/execbuilder/testdata/select b/pkg/sql/opt/exec/execbuilder/testdata/select index 35ec87023f75..7e1d39fa951e 100644 --- a/pkg/sql/opt/exec/execbuilder/testdata/select +++ b/pkg/sql/opt/exec/execbuilder/testdata/select @@ -238,11 +238,11 @@ scan · · () · query TTTTT EXPLAIN (VERBOSE) SELECT rowid FROM [54(3) AS num_ref_alias] ---- -· distribution local · · -· vectorized true · · -scan · · (rowid[hidden]) · -· table num_ref_hidden@primary · · -· spans FULL SCAN · · +· distribution local · · +· vectorized true · · +scan · · (rowid) · +· table num_ref_hidden@primary · · +· spans FULL SCAN · · query error pq: \[666\(1\) AS num_ref_alias\]: relation \"\[666\]\" does not exist EXPLAIN (VERBOSE) SELECT * FROM [666(1) AS num_ref_alias] @@ -400,11 +400,11 @@ scan · · (x, y) · query TTTTT EXPLAIN (VERBOSE) SELECT x, y, rowid FROM b WHERE rowid > 0 ---- -· distribution local · · -· vectorized true · · -scan · · (x, y, rowid[hidden]) · -· table b@primary · · -· spans /1- · · +· distribution local · · +· vectorized true · · +scan · · (x, y, rowid) · +· table b@primary · · +· spans /1- · · statement ok DROP TABLE b diff --git a/pkg/sql/opt/exec/execbuilder/testdata/select_index b/pkg/sql/opt/exec/execbuilder/testdata/select_index index b03391774600..6d5e41b48914 100644 --- a/pkg/sql/opt/exec/execbuilder/testdata/select_index +++ b/pkg/sql/opt/exec/execbuilder/testdata/select_index @@ -966,40 +966,40 @@ CREATE TABLE xy (x INT, y INT, INDEX (y)) query TTTTT EXPLAIN (VERBOSE) SELECT * FROM xy WHERE y IS NOT DISTINCT FROM NULL ---- -· distribution local · · -· vectorized true · · -index-join · · (x, y) · - │ table xy@primary · · - │ key columns rowid · · - └── scan · · (y, rowid[hidden]) · -· table xy@xy_y_idx · · -· spans /NULL-/!NULL · · +· distribution local · · +· vectorized true · · +index-join · · (x, y) · + │ table xy@primary · · + │ key columns rowid · · + └── scan · · (y, rowid) · +· table xy@xy_y_idx · · +· spans /NULL-/!NULL · · query TTTTT EXPLAIN (VERBOSE) SELECT * FROM xy WHERE y IS NOT DISTINCT FROM 4 ---- -· distribution local · · -· vectorized true · · -index-join · · (x, y) · - │ table xy@primary · · - │ key columns rowid · · - └── scan · · (y, rowid[hidden]) · -· table xy@xy_y_idx · · -· spans /4-/5 · · +· distribution local · · +· vectorized true · · +index-join · · (x, y) · + │ table xy@primary · · + │ key columns rowid · · + └── scan · · (y, rowid) · +· table xy@xy_y_idx · · +· spans /4-/5 · · query TTTTT EXPLAIN (VERBOSE) SELECT x FROM xy WHERE y > 0 AND y < 2 ORDER BY y ---- -· distribution local · · -· vectorized true · · -render · · (x) · - │ render 0 x · · - └── index-join · · (x, y) · - │ table xy@primary · · - │ key columns rowid · · - └── scan · · (y, rowid[hidden]) · -· table xy@xy_y_idx · · -· spans /1-/2 · · +· distribution local · · +· vectorized true · · +render · · (x) · + │ render 0 x · · + └── index-join · · (x, y) · + │ table xy@primary · · + │ key columns rowid · · + └── scan · · (y, rowid) · +· table xy@xy_y_idx · · +· spans /1-/2 · · query TTTTT EXPLAIN (VERBOSE) SELECT * FROM xy WHERE y IS DISTINCT FROM NULL diff --git a/pkg/sql/opt/exec/execbuilder/testdata/subquery b/pkg/sql/opt/exec/execbuilder/testdata/subquery index 46e1364323c4..384a890bd010 100644 --- a/pkg/sql/opt/exec/execbuilder/testdata/subquery +++ b/pkg/sql/opt/exec/execbuilder/testdata/subquery @@ -144,18 +144,18 @@ WHERE OR (col4 IN (SELECT col1 FROM tab4 WHERE col1 > 8.27)) AND (col3 <= 5 AND (col3 BETWEEN 7 AND 9)) ---- -· distribution local · · -· vectorized true · · -render · · (col0) · - │ render 0 col0 · · - └── index-join · · (col0, col3, col4, rowid[hidden]) · - │ table tab4@primary · · - │ key columns rowid · · - └── filter · · (col0, col4, rowid[hidden]) · - │ filter col0 <= 0 · · - └── scan · · (col0, col4, rowid[hidden]) · -· table tab4@idx_tab4_0 · · -· spans /!NULL-/5.38/1 · · +· distribution local · · +· vectorized true · · +render · · (col0) · + │ render 0 col0 · · + └── index-join · · (col0, col3, col4, rowid) · + │ table tab4@primary · · + │ key columns rowid · · + └── filter · · (col0, col4, rowid) · + │ filter col0 <= 0 · · + └── scan · · (col0, col4, rowid) · +· table tab4@idx_tab4_0 · · +· spans /!NULL-/5.38/1 · · # ------------------------------------------------------------------------------ # Correlated subqueries. diff --git a/pkg/sql/opt/exec/execbuilder/testdata/tuple b/pkg/sql/opt/exec/execbuilder/testdata/tuple index 89a8f167c922..41decf457ea4 100644 --- a/pkg/sql/opt/exec/execbuilder/testdata/tuple +++ b/pkg/sql/opt/exec/execbuilder/testdata/tuple @@ -87,16 +87,16 @@ CREATE TABLE abc (a INT, b INT, c INT, INDEX(a, b)) query TTTTT EXPLAIN (VERBOSE) SELECT * FROM abc WHERE (a, b, c) > (1, 2, 3) AND (a,b,c) < (8, 9, 10) ---- -· distribution local · · -· vectorized true · · -filter · · (a, b, c) · - │ filter ((a, b, c) > (1, 2, 3)) AND ((a, b, c) < (8, 9, 10)) · · - └── index-join · · (a, b, c) · - │ table abc@primary · · - │ key columns rowid · · - └── scan · · (a, b, rowid[hidden]) · -· table abc@abc_a_b_idx · · -· spans /1/2-/8/10 · · +· distribution local · · +· vectorized true · · +filter · · (a, b, c) · + │ filter ((a, b, c) > (1, 2, 3)) AND ((a, b, c) < (8, 9, 10)) · · + └── index-join · · (a, b, c) · + │ table abc@primary · · + │ key columns rowid · · + └── scan · · (a, b, rowid) · +· table abc@abc_a_b_idx · · +· spans /1/2-/8/10 · · statement ok DROP TABLE abc diff --git a/pkg/sql/opt/exec/execbuilder/testdata/update b/pkg/sql/opt/exec/execbuilder/testdata/update index ca9c8d414d40..f5a010104a6c 100644 --- a/pkg/sql/opt/exec/execbuilder/testdata/update +++ b/pkg/sql/opt/exec/execbuilder/testdata/update @@ -339,37 +339,37 @@ CREATE TABLE abc (a INT, b INT, c INT, INDEX(c) STORING(a,b)) query TTTTT EXPLAIN (VERBOSE) SELECT * FROM [ UPDATE abc SET a=c RETURNING a ] ORDER BY a ---- -· distribution local · · -· vectorized false · · -root · · (a) +a - ├── sort · · (a) +a - │ │ order +a · · - │ └── scan buffer node · · (a) · - │ label buffer 1 · · - └── subquery · · · · - │ id @S1 · · - │ original sql UPDATE abc SET a = c RETURNING a · · - │ exec mode all rows · · - └── buffer node · · (a) · - │ label buffer 1 · · - └── spool · · (a) · - └── render · · (a) · - │ render 0 a · · - └── run · · (a, rowid[hidden]) · - └── update · · (a, rowid[hidden]) · - │ table abc · · - │ set a · · - │ strategy updater · · - └── render · · (a, b, c, rowid, c) · - │ render 0 a · · - │ render 1 b · · - │ render 2 c · · - │ render 3 rowid · · - │ render 4 c · · - └── scan · · (a, b, c, rowid[hidden]) · -· table abc@primary · · -· spans FULL SCAN · · -· locking strength for update · · +· distribution local · · +· vectorized false · · +root · · (a) +a + ├── sort · · (a) +a + │ │ order +a · · + │ └── scan buffer node · · (a) · + │ label buffer 1 · · + └── subquery · · · · + │ id @S1 · · + │ original sql UPDATE abc SET a = c RETURNING a · · + │ exec mode all rows · · + └── buffer node · · (a) · + │ label buffer 1 · · + └── spool · · (a) · + └── render · · (a) · + │ render 0 a · · + └── run · · (a, rowid) · + └── update · · (a, rowid) · + │ table abc · · + │ set a · · + │ strategy updater · · + └── render · · (a, b, c, rowid, c) · + │ render 0 a · · + │ render 1 b · · + │ render 2 c · · + │ render 3 rowid · · + │ render 4 c · · + └── scan · · (a, b, c, rowid) · +· table abc@primary · · +· spans FULL SCAN · · +· locking strength for update · · # ------------------------------------------------------------------------------ # Regression for #35364. This tests behavior that is different between the CBO diff --git a/pkg/sql/opt/exec/execbuilder/testdata/upsert b/pkg/sql/opt/exec/execbuilder/testdata/upsert index fb778e6f2bfe..be586eeec24a 100644 --- a/pkg/sql/opt/exec/execbuilder/testdata/upsert +++ b/pkg/sql/opt/exec/execbuilder/testdata/upsert @@ -365,8 +365,8 @@ root · · └── spool · · (z) · └── render · · (z) · │ render 0 z · · - └── run · · (z, rowid[hidden]) · - └── upsert · · (z, rowid[hidden]) · + └── run · · (z, rowid) · + └── upsert · · (z, rowid) · │ into xyz(x, y, z, rowid) · · │ strategy opt upserter · · └── render · · (a, b, c, column11, a, b, c) · diff --git a/pkg/sql/opt/exec/execbuilder/testdata/with b/pkg/sql/opt/exec/execbuilder/testdata/with index 07757f8f9974..d741c1f2fef8 100644 --- a/pkg/sql/opt/exec/execbuilder/testdata/with +++ b/pkg/sql/opt/exec/execbuilder/testdata/with @@ -59,8 +59,8 @@ root · · └── spool · · (a) · └── render · · (a) · │ render 0 a · · - └── run · · (a, rowid[hidden]) · - └── insert · · (a, rowid[hidden]) · + └── run · · (a, rowid) · + └── insert · · (a, rowid) · │ into x(a, rowid) · · │ strategy inserter · · └── values · · (column1, column5) · diff --git a/pkg/sql/opt/exec/explain/output.go b/pkg/sql/opt/exec/explain/output.go index afac0b323a18..b88f7a114a7e 100644 --- a/pkg/sql/opt/exec/explain/output.go +++ b/pkg/sql/opt/exec/explain/output.go @@ -61,7 +61,7 @@ func (ob *OutputBuilder) EnterNode( ) { var colStr, ordStr string if ob.flags.Verbose { - colStr = columns.String(ob.flags.ShowTypes) + colStr = columns.String(ob.flags.ShowTypes, false /* showHidden */) ordStr = ordering.String(columns) } ob.enterNode(name, colStr, ordStr) diff --git a/pkg/sql/plan_opt_test.go b/pkg/sql/plan_opt_test.go index 31767f272d2f..21145a290a19 100644 --- a/pkg/sql/plan_opt_test.go +++ b/pkg/sql/plan_opt_test.go @@ -90,6 +90,20 @@ func (h *queryCacheTestHelper) AssertStats(tb *testing.T, expHits, expMisses int assert.Equal(tb, expMisses, misses, "misses") } +// CheckStats is similar to AssertStats, but returns an error instead of +// failing the test if the actual stats don't match the expected stats. +func (h *queryCacheTestHelper) CheckStats(tb *testing.T, expHits, expMisses int) error { + tb.Helper() + hits, misses := h.GetStats() + if expHits != hits { + return errors.Errorf("expected %d hits but found %d", expHits, hits) + } + if expMisses != misses { + return errors.Errorf("expected %d misses but found %d", expMisses, misses) + } + return nil +} + func TestQueryCache(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) @@ -347,8 +361,17 @@ SELECT cte.x, cte.y FROM cte LEFT JOIN cte as cte2 on cte.y = cte2.x`, j) h.AssertStats(t, 1 /* hits */, 1 /* misses */) r0.Exec(t, "CREATE STATISTICS s FROM t") h.AssertStats(t, 1 /* hits */, 1 /* misses */) - r1.CheckQueryResults(t, "SELECT * FROM t", [][]string{{"1", "1"}}) - h.AssertStats(t, 1 /* hits */, 2 /* misses */) + hits := 1 + testutils.SucceedsSoon(t, func() error { + // The stats cache is updated asynchronously, so we may get some hits + // before we get a miss. + r1.CheckQueryResults(t, "SELECT * FROM t", [][]string{{"1", "1"}}) + if err := h.CheckStats(t, hits, 2 /* misses */); err != nil { + hits++ + return err + } + return nil + }) }) // Test that a schema change triggers cache invalidation. diff --git a/pkg/sql/planner.go b/pkg/sql/planner.go index 873f0560e324..bc6af2019d4f 100644 --- a/pkg/sql/planner.go +++ b/pkg/sql/planner.go @@ -490,13 +490,7 @@ func (p *planner) LookupTableByID( } return catalog.TableEntry{}, err } - // TODO (rohany): This shouldn't be needed once the descs.Collection always - // returns descriptors with hydrated types. - hydratedDesc, err := p.maybeHydrateTypesInDescriptor(ctx, table) - if err != nil { - return catalog.TableEntry{}, err - } - return catalog.TableEntry{Desc: hydratedDesc.(*sqlbase.ImmutableTableDescriptor)}, nil + return catalog.TableEntry{Desc: table}, nil } // TypeAsString enforces (not hints) that the given expression typechecks as a diff --git a/pkg/sql/resolver.go b/pkg/sql/resolver.go index 081713f9d4c4..fa777f525b06 100644 --- a/pkg/sql/resolver.go +++ b/pkg/sql/resolver.go @@ -16,7 +16,6 @@ import ( "github.com/cockroachdb/cockroach/pkg/keys" "github.com/cockroachdb/cockroach/pkg/kv" - "github.com/cockroachdb/cockroach/pkg/sql/catalog" "github.com/cockroachdb/cockroach/pkg/sql/catalog/resolver" "github.com/cockroachdb/cockroach/pkg/sql/opt/cat" "github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode" @@ -25,7 +24,6 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/sqlbase" "github.com/cockroachdb/cockroach/pkg/sql/types" - "github.com/cockroachdb/cockroach/pkg/util/protoutil" "github.com/cockroachdb/errors" ) @@ -129,14 +127,6 @@ func (p *planner) LookupObject( lookupFlags.CommonLookupFlags = p.CommonLookupFlags(false /* required */) objDesc, err := sc.GetObjectDesc(ctx, p.txn, p.ExecCfg().Settings, p.ExecCfg().Codec, dbName, scName, tbName, lookupFlags) - // The returned object may contain types.T that need hydrating. - if objDesc != nil { - objDesc, err = p.maybeHydrateTypesInDescriptor(ctx, objDesc) - if err != nil { - return false, nil, err - } - } - return objDesc != nil, objDesc, err } @@ -205,38 +195,6 @@ func (p *planner) ResolveTypeByID(ctx context.Context, id uint32) (*types.T, err return desc.MakeTypesT(ctx, name, p) } -// maybeHydrateTypesInDescriptor hydrates any types.T's in the input descriptor. -// TODO (rohany): Once we lease types, this should be pushed down into the -// leased object collection. -func (p *planner) maybeHydrateTypesInDescriptor( - ctx context.Context, objDesc catalog.Descriptor, -) (catalog.Descriptor, error) { - // As of now, only {Mutable,Immutable}TableDescriptor have types.T that - // need to be hydrated. - switch t := objDesc.(type) { - case *sqlbase.MutableTableDescriptor: - // MutableTableDescriptors are safe to modify in place. - if err := sqlbase.HydrateTypesInTableDescriptor(ctx, t.TableDesc(), p); err != nil { - return nil, err - } - return objDesc, nil - case *sqlbase.ImmutableTableDescriptor: - // ImmutableTableDescriptors need to be copied before hydration. If there - // aren't any user defined types in the descriptor, then just return. - if !t.ContainsUserDefinedTypes() { - return objDesc, nil - } - // Make a copy of the underlying TableDescriptor. - desc := protoutil.Clone(t.TableDesc()).(*sqlbase.TableDescriptor) - if err := sqlbase.HydrateTypesInTableDescriptor(ctx, desc, p); err != nil { - return nil, err - } - return sqlbase.NewImmutableTableDescriptor(*desc), nil - default: - return objDesc, nil - } -} - // ObjectLookupFlags is part of the resolver.SchemaResolver interface. func (p *planner) ObjectLookupFlags(required, requireMutable bool) tree.ObjectLookupFlags { return tree.ObjectLookupFlags{ diff --git a/pkg/sql/sqlbase/result_columns.go b/pkg/sql/sqlbase/result_columns.go index e74fc8cf2e5f..3a227e868fc4 100644 --- a/pkg/sql/sqlbase/result_columns.go +++ b/pkg/sql/sqlbase/result_columns.go @@ -103,8 +103,9 @@ func (r ResultColumns) NodeFormatter(colIdx int) tree.NodeFormatter { } // String formats result columns to a string. -// The column types are printed if the argument specifies so. -func (r ResultColumns) String(printTypes bool) string { +// The column types are printed if printTypes is true. +// The hidden property is printed if showHidden is true. +func (r ResultColumns) String(printTypes bool, showHidden bool) string { f := tree.NewFmtCtx(tree.FmtSimple) f.WriteByte('(') for i := range r { @@ -124,7 +125,7 @@ func (r ResultColumns) String(printTypes bool) string { hasProps = true f.WriteString(prop) } - if rCol.Hidden { + if showHidden && rCol.Hidden { outputProp("hidden") } if hasProps { diff --git a/pkg/sql/stats/stats_cache_test.go b/pkg/sql/stats/stats_cache_test.go index 1e0b08c9f8ae..d4e4f1958366 100644 --- a/pkg/sql/stats/stats_cache_test.go +++ b/pkg/sql/stats/stats_cache_test.go @@ -377,7 +377,7 @@ func TestCacheWait(t *testing.T) { before := sc.mu.numInternalQueries id := tableIDs[rand.Intn(len(tableIDs))] - sc.RefreshTableStats(ctx, id) + sc.InvalidateTableStats(ctx, id) // Run GetTableStats multiple times in parallel. var wg sync.WaitGroup for n := 0; n < 10; n++ {