From 8fa41713972e63ecb976062153a4b073864bcb64 Mon Sep 17 00:00:00 2001 From: Yahor Yuzefovich Date: Thu, 7 Dec 2023 11:21:47 -0800 Subject: [PATCH] sql: omit virtual tables from stmt bundles This commit makes it so that we do not include virtual tables into `schema.sql` file (as well as don't create the stats files for them) into the stmt bundle. This should make it easier to recreate the bundles that access virtual tables. Note that in the future, once we support stats on virtual tables, this logic will need to be update so that only the create statements are omitted. Release note: None --- pkg/sql/explain_bundle.go | 4 ++++ pkg/sql/explain_bundle_test.go | 22 ++++++++++++++++++++++ pkg/sql/opt/exec/execbuilder/relational.go | 1 + pkg/sql/opt/metadata.go | 15 ++++++++++++++- 4 files changed, 41 insertions(+), 1 deletion(-) diff --git a/pkg/sql/explain_bundle.go b/pkg/sql/explain_bundle.go index f9b7b7c7eaa0..b82671fd4b7b 100644 --- a/pkg/sql/explain_bundle.go +++ b/pkg/sql/explain_bundle.go @@ -490,6 +490,9 @@ func (b *stmtBundleBuilder) addEnv(ctx context.Context) { } buf.Reset() + // TODO(#27611): when we support stats on virtual tables, we'll need to + // update this logic to not include virtual tables into schema.sql but still + // create stats files for them. var tables, sequences, views []tree.TableName err := b.db.Txn(ctx, func(ctx context.Context, txn *kv.Txn) error { var err error @@ -497,6 +500,7 @@ func (b *stmtBundleBuilder) addEnv(ctx context.Context) { ctx, b.plan.catalog, func(ds cat.DataSource) (cat.DataSourceName, error) { return b.plan.catalog.fullyQualifiedNameWithTxn(ctx, ds, txn) }, + false, /* includeVirtualTables */ ) return err }) diff --git a/pkg/sql/explain_bundle_test.go b/pkg/sql/explain_bundle_test.go index c0f4a3728001..9de78a183179 100644 --- a/pkg/sql/explain_bundle_test.go +++ b/pkg/sql/explain_bundle_test.go @@ -386,6 +386,28 @@ CREATE TABLE users(id UUID DEFAULT gen_random_uuid() PRIMARY KEY, promo_id INT R base, plans, "distsql.html vec.txt vec-v.txt inflight-trace-n1.txt inflight-trace-jaeger-n1.json", ) }) + + t.Run("virtual table", func(t *testing.T) { + rows := r.QueryStr(t, "EXPLAIN ANALYZE (DEBUG) SELECT count(*) FROM pg_catalog.pg_class;") + // tableName is empty since we expect that the table is not included + // into schema.sql. + var tableName string + contentCheck := func(name, contents string) error { + if name != "schema.sql" { + return nil + } + if strings.Contains(contents, "CREATE TABLE pg_catalog.pg_class") { + return errors.New("virtual tables should be omitted from schema.sql") + } + return nil + } + checkBundle( + t, fmt.Sprint(rows), tableName, contentCheck, false, /* expectErrors */ + // Note that the list of files doesn't include stats for the virtual + // table - this will probably change when #27611 is addressed. + base, plans, "distsql.html vec.txt vec-v.txt", + ) + }) } // checkBundle searches text strings for a bundle URL and then verifies that the diff --git a/pkg/sql/opt/exec/execbuilder/relational.go b/pkg/sql/opt/exec/execbuilder/relational.go index eb500818cd1b..c3ba0757a5c1 100644 --- a/pkg/sql/opt/exec/execbuilder/relational.go +++ b/pkg/sql/opt/exec/execbuilder/relational.go @@ -3619,6 +3619,7 @@ func (b *Builder) getEnvData() (exec.ExplainEnvData, error) { func(ds cat.DataSource) (cat.DataSourceName, error) { return b.catalog.FullyQualifiedName(context.TODO(), ds) }, + true, /* includeVirtualTables */ ) return envOpts, err } diff --git a/pkg/sql/opt/metadata.go b/pkg/sql/opt/metadata.go index 744d5c94379e..26f023dfa99d 100644 --- a/pkg/sql/opt/metadata.go +++ b/pkg/sql/opt/metadata.go @@ -987,11 +987,13 @@ func (md *Metadata) getAllReferencedTables( // AllDataSourceNames returns the fully qualified names of all datasources // referenced by the metadata. This includes all tables, sequences, and views // that are directly stored in the metadata, as well as tables that are -// recursively referenced from foreign keys. +// recursively referenced from foreign keys. If includeVirtualTables is false, +// then virtual tables are not returned. func (md *Metadata) AllDataSourceNames( ctx context.Context, catalog cat.Catalog, fullyQualifiedName func(ds cat.DataSource) (cat.DataSourceName, error), + includeVirtualTables bool, ) (tables, sequences, views []tree.TableName, _ error) { // Catalog objects can show up multiple times in our lists, so deduplicate // them. @@ -1014,6 +1016,17 @@ func (md *Metadata) AllDataSourceNames( } var err error refTables := md.getAllReferencedTables(ctx, catalog) + if !includeVirtualTables { + // Update refTables in-place to remove all virtual tables. + i := 0 + for j := 0; j < len(refTables); j++ { + if t, ok := refTables[j].(cat.Table); !ok || !t.IsVirtualTable() { + refTables[i] = refTables[j] + i++ + } + } + refTables = refTables[:i] + } tables, err = getNames(len(refTables), func(i int) cat.DataSource { return refTables[i] })