From c44619c38ba6f22d8d6590a34600ba0c2fd7fdd9 Mon Sep 17 00:00:00 2001 From: Jordan Lewis Date: Fri, 10 Apr 2020 00:12:03 -0400 Subject: [PATCH] sql: SHOW CREATE takes advantage of virtual index This commit updates the implementation of SHOW CREATE so that it doesn't have to filter every single table in the database. It does 2 things: 1. Stop outputting zone configs in crdb_internal.create_statements 2. Start outputting partitioning status in the above instead 3. Get zone configs from the SQL query in SHOW CREATE with a subquery from the zone configs table and join together in SQL This strategy makes it possible to give crdb_internal.create_statements a virtual index. Now SHOW CREATE is really fast! Release note (performance improvement): SHOW CREATE is much more efficient --- pkg/sql/crdb_internal.go | 209 +++++++----------- pkg/sql/delegate/show_table.go | 32 ++- .../testdata/logic_test/crdb_internal | 2 +- .../logictest/testdata/logic_test/sequences | 6 +- pkg/sql/partition_test.go | 5 + 5 files changed, 103 insertions(+), 151 deletions(-) diff --git a/pkg/sql/crdb_internal.go b/pkg/sql/crdb_internal.go index 3c4e725d8be8..19cb048c5d9e 100644 --- a/pkg/sql/crdb_internal.go +++ b/pkg/sql/crdb_internal.go @@ -47,7 +47,6 @@ import ( "github.com/cockroachdb/cockroach/pkg/util/protoutil" "github.com/cockroachdb/cockroach/pkg/util/timeutil" "github.com/cockroachdb/errors" - "gopkg.in/yaml.v2" ) const crdbInternalName = sessiondata.CRDBInternalSchemaName @@ -1372,13 +1371,18 @@ CREATE TABLE crdb_internal.builtin_functions ( }, } +// Prepare the row populate function. +var typeView = tree.NewDString("view") +var typeTable = tree.NewDString("table") +var typeSequence = tree.NewDString("sequence") + // crdbInternalCreateStmtsTable exposes the CREATE TABLE/CREATE VIEW // statements. // // TODO(tbg): prefix with kv_. -var crdbInternalCreateStmtsTable = virtualSchemaTable{ - comment: `CREATE and ALTER statements for all tables accessible by current user in current database (KV scan)`, - schema: ` +var crdbInternalCreateStmtsTable = makeAllRelationsVirtualTableWithDescriptorIDIndex( + `CREATE and ALTER statements for all tables accessible by current user in current database (KV scan)`, + ` CREATE TABLE crdb_internal.create_statements ( database_id INT, database_name STRING, @@ -1391,148 +1395,83 @@ CREATE TABLE crdb_internal.create_statements ( create_nofks STRING NOT NULL, alter_statements STRING[] NOT NULL, validate_statements STRING[] NOT NULL, - zone_configuration_statements STRING[] NOT NULL + has_partitions BOOL NOT NULL, + INDEX(descriptor_id) ) -`, - populate: func(ctx context.Context, p *planner, dbContext *DatabaseDescriptor, addRow func(...tree.Datum) error) error { +`, virtualOnce, false, /* includesIndexEntries */ + func(ctx context.Context, p *planner, h oidHasher, db *sqlbase.DatabaseDescriptor, scName string, + table *sqlbase.TableDescriptor, lookup simpleSchemaResolver, addRow func(...tree.Datum) error) error { contextName := "" - if dbContext != nil { - contextName = dbContext.Name - } - - // Prepare the row populate function. - typeView := tree.NewDString("view") - typeTable := tree.NewDString("table") - typeSequence := tree.NewDString("sequence") - - // Hold the configuration statements for each table - zoneConfigStmts := make(map[string][]string) - // Prepare a query used to see zones configuations on this table. - configStmtsQuery := ` - SELECT - table_name, raw_config_yaml, raw_config_sql - FROM - crdb_internal.zones - WHERE - database_name = '%[1]s' - AND table_name IS NOT NULL - AND raw_config_yaml IS NOT NULL - AND raw_config_sql IS NOT NULL - ORDER BY - database_name, table_name, index_name, partition_name - ` - // The create_statements table is used at times where other internal - // tables have not been created, or are unaccessible (perhaps during - // certain tests (TestDumpAsOf in pkg/cli/dump_test.go)). So if something - // goes wrong querying this table, proceed without any constraint data. - zoneConstraintRows, err := p.ExtendedEvalContext().ExecCfg.InternalExecutor.Query( - ctx, "zone-constraints-for-show-create-table", p.txn, - fmt.Sprintf(configStmtsQuery, contextName)) - if err != nil { - log.VEventf(ctx, 1, "%q", err) + parentNameStr := tree.DNull + if db != nil { + contextName = db.Name + parentNameStr = tree.NewDString(db.Name) + } + scNameStr := tree.NewDString(scName) + + var descType tree.Datum + var stmt, createNofk string + alterStmts := tree.NewDArray(types.String) + validateStmts := tree.NewDArray(types.String) + var err error + if table.IsView() { + descType = typeView + stmt, err = ShowCreateView(ctx, (*tree.Name)(&table.Name), table) + } else if table.IsSequence() { + descType = typeSequence + stmt, err = ShowCreateSequence(ctx, (*tree.Name)(&table.Name), table) } else { - for _, row := range zoneConstraintRows { - tableName := string(tree.MustBeDString(row[0])) - var zoneConfig zonepb.ZoneConfig - yamlString := string(tree.MustBeDString(row[1])) - err := yaml.UnmarshalStrict([]byte(yamlString), &zoneConfig) - if err != nil { - return err - } - // If all constraints are default, then don't show anything. - if !zoneConfig.Equal(zonepb.ZoneConfig{}) { - sqlString := string(tree.MustBeDString(row[2])) - zoneConfigStmts[tableName] = append(zoneConfigStmts[tableName], sqlString) - } + descType = typeTable + tn := (*tree.Name)(&table.Name) + createNofk, err = ShowCreateTable(ctx, p, tn, contextName, table, lookup, OmitFKClausesFromCreate) + if err != nil { + return err + } + allIdx := append(table.Indexes, table.PrimaryIndex) + if err := showAlterStatementWithInterleave(ctx, tn, contextName, lookup, allIdx, table, alterStmts, + validateStmts); err != nil { + return err } + stmt, err = ShowCreateTable(ctx, p, tn, contextName, table, lookup, IncludeFkClausesInCreate) + } + if err != nil { + return err } - return forEachTableDescWithTableLookupInternal(ctx, p, dbContext, virtualOnce, true, /*allowAdding*/ - func(db *DatabaseDescriptor, scName string, table *TableDescriptor, lCtx tableLookupFn) error { - parentNameStr := tree.DNull - if db != nil { - parentNameStr = tree.NewDString(db.Name) - } - scNameStr := tree.NewDString(scName) - - var descType tree.Datum - var stmt, createNofk string - alterStmts := tree.NewDArray(types.String) - validateStmts := tree.NewDArray(types.String) - var err error - if table.IsView() { - descType = typeView - stmt, err = ShowCreateView(ctx, (*tree.Name)(&table.Name), table) - } else if table.IsSequence() { - descType = typeSequence - stmt, err = ShowCreateSequence(ctx, (*tree.Name)(&table.Name), table) - } else { - descType = typeTable - tn := (*tree.Name)(&table.Name) - createNofk, err = ShowCreateTable(ctx, p, tn, contextName, table, lCtx, OmitFKClausesFromCreate) - if err != nil { - return err - } - allIdx := append(table.Indexes, table.PrimaryIndex) - if err := showAlterStatementWithInterleave(ctx, tn, contextName, lCtx, allIdx, table, alterStmts, validateStmts); err != nil { - return err - } - stmt, err = ShowCreateTable(ctx, p, tn, contextName, table, lCtx, IncludeFkClausesInCreate) - } - if err != nil { - return err - } - - zoneRows := tree.NewDArray(types.String) - if val, ok := zoneConfigStmts[table.Name]; ok { - for _, s := range val { - if err := zoneRows.Append(tree.NewDString(s)); err != nil { - return err - } - } - } else { - // If there are partitions applied to this table and no zone configurations, display a warning. - hasPartitions := false - for i := range table.Indexes { - if table.Indexes[i].Partitioning.NumColumns != 0 { - hasPartitions = true - break - } - } - hasPartitions = hasPartitions || table.PrimaryIndex.Partitioning.NumColumns != 0 - if hasPartitions { - stmt += "\n-- Warning: Partitioned table with no zone configurations." - } - } - - descID := tree.NewDInt(tree.DInt(table.ID)) - dbDescID := tree.NewDInt(tree.DInt(table.GetParentID())) - if createNofk == "" { - createNofk = stmt - } - return addRow( - dbDescID, - parentNameStr, - scNameStr, - descID, - descType, - tree.NewDString(table.Name), - tree.NewDString(stmt), - tree.NewDString(table.State.String()), - tree.NewDString(createNofk), - alterStmts, - validateStmts, - zoneRows, - ) - }) - }, -} + descID := tree.NewDInt(tree.DInt(table.ID)) + dbDescID := tree.NewDInt(tree.DInt(table.GetParentID())) + if createNofk == "" { + createNofk = stmt + } + hasPartitions := false + for i := range table.Indexes { + if table.Indexes[i].Partitioning.NumColumns != 0 { + hasPartitions = true + break + } + } + hasPartitions = hasPartitions || table.PrimaryIndex.Partitioning.NumColumns != 0 + return addRow( + dbDescID, + parentNameStr, + scNameStr, + descID, + descType, + tree.NewDString(table.Name), + tree.NewDString(stmt), + tree.NewDString(table.State.String()), + tree.NewDString(createNofk), + alterStmts, + validateStmts, + tree.MakeDBool(tree.DBool(hasPartitions)), + ) + }) func showAlterStatementWithInterleave( ctx context.Context, tn *tree.Name, contextName string, - lCtx tableLookupFn, + lCtx simpleSchemaResolver, allIdx []sqlbase.IndexDescriptor, table *sqlbase.TableDescriptor, alterStmts *tree.DArray, diff --git a/pkg/sql/delegate/show_table.go b/pkg/sql/delegate/show_table.go index 5d8904bcbd95..c672c4203d0c 100644 --- a/pkg/sql/delegate/show_table.go +++ b/pkg/sql/delegate/show_table.go @@ -21,23 +21,30 @@ import ( func (d *delegator) delegateShowCreate(n *tree.ShowCreate) (tree.Statement, error) { sqltelemetry.IncrementShowCounter(sqltelemetry.Create) + const showCreateQuery = ` + WITH zone_configs AS ( + SELECT string_agg(raw_config_sql, e';\n') FROM crdb_internal.zones + WHERE database_name = %[1]s + AND table_name = %[2]s + AND raw_config_yaml IS NOT NULL + AND raw_config_sql IS NOT NULL + ) SELECT %[3]s AS table_name, - array_to_string( - array_cat( - ARRAY[create_statement], - zone_configuration_statements - ), - e';\n' - ) - AS create_statement + concat(create_statement, + CASE + WHEN NOT has_partitions + THEN NULL + WHEN (SELECT * FROM zone_configs) IS NULL + THEN e'\n-- Warning: Partitioned table with no zone configurations.' + ELSE concat(e';\n', (SELECT * FROM zone_configs)) + END + ) AS create_statement FROM %[4]s.crdb_internal.create_statements WHERE - (database_name IS NULL OR database_name = %[1]s) - AND schema_name = %[5]s - AND descriptor_name = %[2]s + descriptor_id = %[6]d ` return d.showTableDetails(n.Name, showCreateQuery) @@ -164,6 +171,7 @@ func (d *delegator) delegateShowConstraints(n *tree.ShowConstraints) (tree.State // %[3]s the given table name as SQL string literal. // %[4]s the database name as SQL identifier. // %[5]s the schema name as SQL string literal. +// %[6]s the table ID. func (d *delegator) showTableDetails( name *tree.UnresolvedObjectName, query string, ) (tree.Statement, error) { @@ -185,7 +193,7 @@ func (d *delegator) showTableDetails( lex.EscapeSQLString(resName.String()), resName.CatalogName.String(), // note: CatalogName.String() != Catalog() lex.EscapeSQLString(resName.Schema()), - dataSource.ID(), + dataSource.PostgresDescriptorID(), ) return parse(fullQuery) diff --git a/pkg/sql/logictest/testdata/logic_test/crdb_internal b/pkg/sql/logictest/testdata/logic_test/crdb_internal index d287bebf384f..14943c69bb35 100644 --- a/pkg/sql/logictest/testdata/logic_test/crdb_internal +++ b/pkg/sql/logictest/testdata/logic_test/crdb_internal @@ -187,7 +187,7 @@ function signature category details query ITTITTTTTTTT colnames SELECT * FROM crdb_internal.create_statements WHERE database_name = '' ---- -database_id database_name schema_name descriptor_id descriptor_type descriptor_name create_statement state create_nofks alter_statements validate_statements zone_configuration_statements +database_id database_name schema_name descriptor_id descriptor_type descriptor_name create_statement state create_nofks alter_statements validate_statements has_partitions query ITITTBTB colnames SELECT * FROM crdb_internal.table_columns WHERE descriptor_name = '' diff --git a/pkg/sql/logictest/testdata/logic_test/sequences b/pkg/sql/logictest/testdata/logic_test/sequences index b8b55d407804..f7e2e7681ede 100644 --- a/pkg/sql/logictest/testdata/logic_test/sequences +++ b/pkg/sql/logictest/testdata/logic_test/sequences @@ -113,11 +113,11 @@ CREATE SEQUENCE ignored_options_test CACHE 1 NO CYCLE statement ok CREATE SEQUENCE show_create_test -query ITTITTTTTTTT colnames +query ITTITTTTTTTB colnames SELECT * FROM crdb_internal.create_statements WHERE descriptor_name = 'show_create_test' ---- -database_id database_name schema_name descriptor_id descriptor_type descriptor_name create_statement state create_nofks alter_statements validate_statements zone_configuration_statements -52 test public 66 sequence show_create_test CREATE SEQUENCE show_create_test MINVALUE 1 MAXVALUE 9223372036854775807 INCREMENT 1 START 1 PUBLIC CREATE SEQUENCE show_create_test MINVALUE 1 MAXVALUE 9223372036854775807 INCREMENT 1 START 1 {} {} {} +database_id database_name schema_name descriptor_id descriptor_type descriptor_name create_statement state create_nofks alter_statements validate_statements has_partitions +52 test public 66 sequence show_create_test CREATE SEQUENCE show_create_test MINVALUE 1 MAXVALUE 9223372036854775807 INCREMENT 1 START 1 PUBLIC CREATE SEQUENCE show_create_test MINVALUE 1 MAXVALUE 9223372036854775807 INCREMENT 1 START 1 {} {} false query TT colnames SHOW CREATE SEQUENCE show_create_test diff --git a/pkg/sql/partition_test.go b/pkg/sql/partition_test.go index 27767caed98d..a373f38cb02e 100644 --- a/pkg/sql/partition_test.go +++ b/pkg/sql/partition_test.go @@ -60,9 +60,14 @@ func TestRemovePartitioningOSS(t *testing.T) { ToExclusive: encoding.EncodeIntValue(nil /* appendTo */, encoding.NoColumnID, 2), }}, } + // Note that this is really a gross hack - it breaks planner caches, which + // assume that nothing is going to change out from under them like this. We + // "fix" the issue by altering the table's name to refresh the cache, below. if err := kvDB.Put(ctx, tableKey, sqlbase.WrapDescriptor(tableDesc)); err != nil { t.Fatal(err) } + sqlDB.Exec(t, "ALTER TABLE t.kv RENAME to t.kv2") + sqlDB.Exec(t, "ALTER TABLE t.kv2 RENAME to t.kv") exp := `CREATE TABLE kv ( k INT8 NOT NULL, v INT8 NULL,