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

opt: add partition info to the opt catalog #60818

Merged
merged 1 commit into from
Feb 20, 2021

Conversation

rytaft
Copy link
Collaborator

@rytaft rytaft commented Feb 19, 2021

Prior to this commit, the opt catalog did not include zone information
or prefixes specific to each partition of an index. This commit adds this
information since it will be necessary to support locality optimized
search in a future commit.

Informs #55185

Release note: None

@rytaft rytaft requested review from mgartner, RaduBerinde and a team February 19, 2021 21:35
@rytaft rytaft requested a review from a team as a code owner February 19, 2021 21:35
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Member

@RaduBerinde RaduBerinde 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 @mgartner and @rytaft)


pkg/sql/opt/cat/index.go, line 190 at r1 (raw file):

	//   [ /us/seattle\x00 -               ]
	//
	PartitionByListPrefixes() []tree.Datums

Shouldn't we remove this now? We can get the same by joining the prefixes for all the Partitions, no?

Prior to this commit, the opt catalog did not include zone information
or prefixes specific to each partition of an index. This commit adds this
information since it will be necessary to support locality optimized
search in a future commit.

Informs cockroachdb#55185

Release note: None
Copy link
Collaborator Author

@rytaft rytaft 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 @mgartner)


pkg/sql/opt/cat/index.go, line 190 at r1 (raw file):

Previously, RaduBerinde wrote…

Shouldn't we remove this now? We can get the same by joining the prefixes for all the Partitions, no?

Done. Also moved some of this comment down.

Copy link
Collaborator

@mgartner mgartner 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 3 of 13 files at r1, 11 of 11 files at r2.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained

@rytaft
Copy link
Collaborator Author

rytaft commented Feb 20, 2021

TFTR!

bors r+

@craig
Copy link
Contributor

craig bot commented Feb 20, 2021

Build succeeded:

@craig craig bot merged commit 8b6f3c8 into cockroachdb:master Feb 20, 2021
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.

4 participants