Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sql: add interleaved_indexes/interleaved_tables into crdb_internal #62076

Merged
merged 1 commit into from
Mar 19, 2021

Conversation

fqazi
Copy link
Collaborator

@fqazi fqazi commented Mar 16, 2021

Previous, when we added the crdb_internal.interleaved table that
showed all interleaved indexes, however it was impossible tell
which table the primary key of the parent table was interleaved.
This was inadequate because users could not tell what the parent
table was. To address this, this patch removes
crdb_internal.interleaved and converts it into two virtual tables
interlaved_indexes/interleaved_tables where the one is for
non-primary key indexes and the second is for tables interleaved
on the primary key.

Release note (sql change): Renamed crdb_internal.interleaved to
crdb_internal.interlaved_indexes and added crdb_internal.interlaved_table
for viewing interleaved tables on the primary key.

@fqazi fqazi requested a review from a team March 16, 2021 14:29
@fqazi fqazi requested a review from a team as a code owner March 16, 2021 14:29
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Contributor

@ajwerner ajwerner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


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

					tree.NewDString(parentSchemaName),
					tree.NewDString(table.GetName()),
					tree.NewDString(parentTable.GetName())); err != nil {

shouldn't we post the parent's schema and database too?

Copy link
Collaborator Author

@fqazi fqazi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


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

Previously, ajwerner wrote…

shouldn't we post the parent's schema and database too?

Done. That was silly, I had it in my original changes but forgot it for some reason.

@fqazi fqazi requested a review from ajwerner March 17, 2021 13:18
Copy link
Contributor

@ajwerner ajwerner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the case we're now not covering well is the case where a secondary index is interleaved into another table, especially one in a different schema. This makes me think we need the database and schema on both tables. I feel like we're coming around full circle...

andrew@free-tier.gcp-us-central1.crdb.io:26257/defaultdb> CREATE TABLE parent (k STRING PRIMARY KEY);
CREATE TABLE

Time: 30ms total (execution 27ms / network 3ms)

andrew@free-tier.gcp-us-central1.crdb.io:26257/defaultdb> CREATE SCHEMA child_schema;
CREATE SCHEMA

Time: 159ms total (execution 29ms / network 130ms)

andrew@free-tier.gcp-us-central1.crdb.io:26257/defaultdb> CREATE TABLE child_schema.child (ck INT PRIMARY KEY, k STRING UNIQUENOT NULL 
CREATE TABLE

Time: 392ms total (execution 69ms / network 323ms)

andrew@free-tier.gcp-us-central1.crdb.io:26257/defaultdb> CREATE INDEX interl ON child_schema.child (k) INTERLEAVE IN PARENT pa
rent(k);
CREATE INDEX

Time: 749ms total (execution 93ms / network 656ms)

Reviewed 1 of 10 files at r1, 1 of 3 files at r2.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @fqazi)

@fqazi fqazi force-pushed the InterleavedTbl branch 2 times, most recently from 3875a5b to 28da19e Compare March 18, 2021 13:21
@fqazi fqazi requested a review from ajwerner March 18, 2021 13:23
Previous, when we added the crdb_internal.interleaved table that
showed all interleaved indexes, however it was impossible tell
which table the primary key of the parent table was interleaved.
This was inadequate because users could not tell what the parent
table was. To address this, this patch modifies
crdb_internal.interleaved to add a parent_table_name column
replacing the parent_index one, since that column could only be
the primary key.

Release note (sql change): Updated crdb_internal.interleaved to
add the parent_table_name column replacing the parent_index_name
column.
Copy link
Contributor

@ajwerner ajwerner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 8 of 10 files at r3.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @fqazi)

@fqazi
Copy link
Collaborator Author

fqazi commented Mar 19, 2021

bors r=ajwerner

@craig
Copy link
Contributor

craig bot commented Mar 19, 2021

Build succeeded:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants