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: alias @primary to PRIMARY KEY #72534

Merged
merged 1 commit into from
Nov 9, 2021

Conversation

otan
Copy link
Contributor

@otan otan commented Nov 8, 2021

Release note (sql change): PRIMARY KEYs have been renamed to conform
to postgres (e.g. @tbl_col1_col2_pkey) in this release. To protect
certain use cases of backwards compatability, we also allow @primary
index hints to alias to the PRIMARY KEY, but only if no other index is
named primary.

@otan otan requested a review from RaduBerinde November 8, 2021 20:30
@otan otan requested a review from a team as a code owner November 8, 2021 20:30
@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.

:lgtm: with some nits. Thanks!

[nit] "compatibility" in the release note

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @otan and @RaduBerinde)


pkg/sql/logictest/testdata/logic_test/table, line 862 at r1 (raw file):

  id INT PRIMARY KEY,
  b INT
); CREATE TABLE tbl_with_primary_named_index (

[nit] this is hard to read, just use two statement ok


pkg/sql/logictest/testdata/logic_test/table, line 883 at r1 (raw file):


query T
EXPLAIN SELECT b FROM tbl_with_primary_named_index@primary

[nit] we try to keep EXPLAIN out of logictests, maybe this should be an execbuilder test?


pkg/sql/logictest/testdata/logic_test/table, line 890 at r1 (raw file):

• scan
  missing stats
  table: tbl_with_primary_named_index@primary

This is not conclusive evidence that we're using the secondary index. Maybe add ORDER BY b or SELECT * (to force an index join)


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

		}
		// Fallback to referencing @primary as the PRIMARY KEY.
		// Note indexes with "primary" as their name takes precedence above.

[nit] note that or "Note: "

@otan otan force-pushed the primary_alias branch 2 times, most recently from f49f0af to 21386c6 Compare November 8, 2021 21:20
@otan
Copy link
Contributor Author

otan commented Nov 9, 2021

bors r=raduberinde

@craig
Copy link
Contributor

craig bot commented Nov 9, 2021

Build failed:

Release note (sql change): PRIMARY KEYs have been renamed to conform
to postgres (e.g. `@tbl_col1_col2_pkey`) in this release. To protect
certain use cases of backwards compatibility, we also allow `@primary`
index hints to alias to the PRIMARY KEY, but only if no other index is
named `primary`.
@otan
Copy link
Contributor Author

otan commented Nov 9, 2021

bors r=raduberinde

@craig
Copy link
Contributor

craig bot commented Nov 9, 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