sql: add internal tables cross db refs and interleaved indexes/tables

[fqazi]

Fixes: #58867

Previously, users had no way of determining which objects in
their database utilized either deprecated features like
interleaved indexes/tables or cross DB references. This was
inadequate since users need to know which object utilize
this functionality. To address this, this patch introduces
the crdb_internal tables: cross_db_references, interleaved_indexes,
interleaved_tables.

Release justification: Low risk internal tables for detecting
deprecated features.
Release note (sql change): Added crdb_internal tables cross_db_references,
interleaved_indexes, and interleaved_tables for detecting the
deprecated features cross db references and interleaved tables
/ indexes within a given database.

[cockroach-teamcity]

This change is 

[ajwerner]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @fqazi)

---

pkg/cli/zip_test.go, line 76 at r1 (raw file):

Quoted 6 lines of code…

  'cross_db_references',
	'databases',
	'forward_dependencies',
	'index_columns',
  'interleaved_indexes',
  'interleaved_tables',

I think this needs a gofmt

---

pkg/sql/crdb_internal.go, line 4034 at r1 (raw file):

}

var crdbInternalInterleavedTables = virtualSchemaTable{

Is this table useful? I worry that it might be confusing. When would you ever want this and not interleaved_indexes?

---

pkg/sql/crdb_internal.go, line 4042 at r1 (raw file):

	parent_table  STRING NOT NULL
)`,
	populate: func(ctx context.Context, p *planner, dbContext *dbdesc.Immutable, addRow func(...tree.Datum) error) error {

I feel like this dbContext may surprise you. Are you familiar with the "".crdb_internal.interleaved_tables weird syntax we have for all databases. I think it's just something to be aware of. I suspect that we're going to end up recommending it so we should probably focus on it.

---

pkg/sql/crdb_internal.go, line 4045 at r1 (raw file):

		return forEachTableDescAll(ctx, p, dbContext, hideVirtual,
			func(db *dbdesc.Immutable, schemaName string, table catalog.TableDescriptor) error {
				if table.IsInterleaved() {

total and complete nit: invert this condition and early return? I feel like the code base tends to prefer that pattern.

---

pkg/sql/crdb_internal.go, line 4050 at r1 (raw file):

						ancestor := index.GetInterleaveAncestor(index.NumInterleaveAncestors() - 1)
						parentID := ancestor.TableID
						parentTable, err := p.Descriptors().GetImmutableTableByID(ctx, p.txn, parentID,

maybe forEachTableDescWithTableLookup which will use the already fetched descriptors. Another thing is that we probably want to look up the parent's database and schema.

[fqazi]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @ajwerner and @fqazi)

---

pkg/sql/crdb_internal.go, line 4034 at r1 (raw file):

Previously, ajwerner wrote…

Is this table useful? I worry that it might be confusing. When would you ever want this and not interleaved_indexes?

Done.

---

pkg/sql/crdb_internal.go, line 4050 at r1 (raw file):

Previously, ajwerner wrote…

maybe forEachTableDescWithTableLookup which will use the already fetched descriptors. Another thing is that we probably want to look up the parent's database and schema.

Done.

[ajwerner]

Reviewed 6 of 13 files at r2.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @fqazi)

---

pkg/sql/crdb_internal.go, line 4052 at r2 (raw file):

					if index.NumInterleaveAncestors() == 0 {
						return nil
					}

should this be continue? Maybe worth adding a test that would have caught this by having a table with some indexes which are not interleaved?

Perhaps built yourself a closure to invoke per index (I don't feel strongly about that suggestion).

---

pkg/sql/logictest/testdata/logic_test/pg_catalog, line 2102 at r2 (raw file):

4294967277  4294967189  0         index columns for all indexes accessible by current user in current database (KV scan)
4294967247  4294967189  0         virtual table with interleaved index information
4294967248  4294967189  0         virtual table with interleaved table information

seems stale, may need a --rewrite

---

pkg/sql/logictest/testdata/logic_test/views, line 905 at r2 (raw file):

statement ok
DROP TABLE t

can you check on views which reference types, sequences, and views in other databases?

[fqazi]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @ajwerner)

---

pkg/sql/crdb_internal.go, line 4052 at r2 (raw file):

Previously, ajwerner wrote…

					if index.NumInterleaveAncestors() == 0 {
						return nil
					}

should this be continue? Maybe worth adding a test that would have caught this by having a table with some indexes which are not interleaved?

Perhaps built yourself a closure to invoke per index (I don't feel strongly about that suggestion).

Yes should have been a continue. Done

---

pkg/sql/logictest/testdata/logic_test/pg_catalog, line 2102 at r2 (raw file):

Previously, ajwerner wrote…

seems stale, may need a --rewrite

Done.

---

pkg/sql/logictest/testdata/logic_test/views, line 905 at r2 (raw file):

Previously, ajwerner wrote…

can you check on views which reference types, sequences, and views in other databases?

Done.

[ajwerner]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @ajwerner and @fqazi)

---

pkg/sql/crdb_internal.go, line 4092 at r3 (raw file):

	comment: `virtual table with cross db references`,
	schema: `
CREATE TABLE crdb_internal.cross_db_references (

nit: this is ugly.

---

pkg/sql/crdb_internal.go, line 4174 at r4 (raw file):

					// For sequences check if the sequence is owned by
					// a different database.

I think we also want to check the set of used descriptors. Say you do:

CREATE DATABASE db;
CREATE SEQUENCE db.s;
CREATE TABLE t (i INT PRIMARY KEY DEFAULT (nextval('db.s')));

We'll have a use reference but not an ownership.

[fqazi]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @ajwerner)

---

pkg/sql/crdb_internal.go, line 4174 at r4 (raw file):

Previously, ajwerner wrote…

					// For sequences check if the sequence is owned by
					// a different database.

I think we also want to check the set of used descriptors. Say you do:

CREATE DATABASE db;
CREATE SEQUENCE db.s;
CREATE TABLE t (i INT PRIMARY KEY DEFAULT (nextval('db.s')));

We'll have a use reference but not an ownership.

Done.

[ajwerner]

Unfortunately I think you've got some more tests to rewrite due to the new crdb_internal tables. I'm happy when CI is happy.

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @ajwerner and @fqazi)

---

pkg/sql/crdb_internal.go, line 4129 at r5 (raw file):

Quoted 9 lines of code…

								if referencedTable.GetParentSchemaID() == keys.PublicSchemaID {
									refSchemaName = tree.PublicSchema
								} else {
									schema, err := lookupFn.getSchemaByID(referencedTable.GetParentSchemaID())
									if err != nil {
										return err
									}
									refSchemaName = schema.GetName()
								}

factor out the schema name retrieval onto the internalLookupCtx? That code shows up like 5 times.

[fqazi]

bors r+

[craig]

Build succeeded:

• GitHub CI (Cockroach)