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.

Sign up for GitHub

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

Jump to bottom

sql: add CREATE … NOT VISIBLE to parser #84783

Merged

craig merged 1 commit into cockroachdb:master from wenyihu6:2-add-CREATE-INVISIBLE-new

Jul 30, 2022

Contributor

wenyihu6 commented Jul 21, 2022 •

edited

Loading

This commit adds parsing support for CREATE INDEX … NOT VISIBLE
and CREATE TABLE (... INDEX() NOT VISIBLE).

This commit does not add any logic to the optimizer, and executing it returns an
“unimplemented” error immediately.

Assists: #72576

See also: #84912

Release note (sql change): Parser now supports creating an index with the option
to mark them as invisible. But no implementation has done yet, and executing it
returns an “unimplemented” error immediately.

Member

cockroach-teamcity commented Jul 21, 2022

This change is

wenyihu6 mentioned this pull request

sql: added CREATE … NOT VISIBLE to parser #83471

Closed

wenyihu6 marked this pull request as ready for review

July 21, 2022 03:59

wenyihu6 requested a review from a team as a code owner

July 21, 2022 03:59

wenyihu6 requested review from a team

July 21, 2022 03:59

wenyihu6 requested review from a team as code owners

July 21, 2022 03:59

Contributor Author

wenyihu6 commented Jul 21, 2022 •

edited

Loading

The first commit comes from #84763. The second commit comes from #84776. Update: the first commit has been merged into master, and this branch has been rebased.

wenyihu6 changed the title ~~sql: added CREATE … NOT VISIBLE to parser~~ sql: add CREATE … NOT VISIBLE to parser

knz reviewed

View reviewed changes

pkg/sql/delegate/show_database_indexes.go Outdated

@@ @@ -28,7 +36,8 @@ SELECT @@
               	column_name,
               	direction,
               	storing::BOOL,
-              	implicit::BOOL`
+              	implicit::BOOL,
+                is_visible::BOOL AS visible`

Contributor

knz Jul 21, 2022

nit: use tabs not spaces for consistency

(alternatively, replace the other tabs with spaces)

pkg/sql/delegate/show_table.go Outdated

@@ @@ -135,7 +138,7 @@ WHERE @@
                   AND table_schema=%[5]s
                   AND table_name=%[2]s
               ORDER BY
-, 2, 3, 4, 5, 6, 7, 8;`
+, 2, 4;`

Contributor

knz Jul 21, 2022

was it on purpose you removed the other ordering columns?

Looks to me like this will make the output non-deterministic.

Collaborator

michae2 Jul 21, 2022 •

edited

Loading

I asked Wenyi to make this change (here). Columns 1, 2, 4 are the natural key of the query, and will be unique.

pkg/sql/opt/testutils/testcat/create_table.go Outdated

@@ @@ -704,6 +704,10 @@ func (tt *Table) addIndexWithVersion( @@
               		tt.addUniqueConstraint(def.Name, def.Columns, def.Predicate, false /* withoutIndex */)
               	}
+              	if def.Hidden {
+              		panic("unimplemented: creating an invisible index is not supported yet.")

Contributor

knz Jul 21, 2022

nit: panic(unimplemented.New(...))

pkg/sql/sem/tree/create.go Outdated

@@ @@ -232,6 +232,7 @@ type CreateIndex struct { @@
               	StorageParams    StorageParams
               	Predicate        Expr
               	Concurrently     bool
+              	Hidden           bool

Contributor

knz Jul 21, 2022

Rename this to NotVisible. Your future self will thank you.

pkg/sql/sem/tree/create.go Outdated

@@ @@ -978,6 +982,7 @@ type IndexTableDef struct { @@
               	PartitionByIndex *PartitionByIndex
               	StorageParams    StorageParams
               	Predicate        Expr
+              	Hidden           bool

Contributor

knz Jul 21, 2022

ditto

wenyihu6 force-pushed the 2-add-CREATE-INVISIBLE-new branch 3 times, most recently from 9ad7242 to eb99a47 Compare

July 21, 2022 12:54

Contributor Author

wenyihu6 commented Jul 21, 2022

pkg/sql/delegate/show_database_indexes.go line 40 at r3 (raw file):

Previously, knz (kena) wrote…

nit: use tabs not spaces for consistency

(alternatively, replace the other tabs with spaces)

Done.

Contributor Author

wenyihu6 commented Jul 21, 2022

pkg/sql/delegate/show_table.go line 141 at r3 (raw file):

Previously, knz (kena) wrote…

was it on purpose you removed the other ordering columns?

Looks to me like this will make the output non-deterministic.

I made the change because I thought it would be deterministic if I order them by table_name, index_name, and then seq_in_index as well. Some test cases were failing if they have SELECT c1, c2 FROM [SHOW INDEX FROM ...] without ORDER BY index_name, seq_in_index following them. There could be different values of c2 with same c1, and I was trying to solve it by this.

wenyihu6 force-pushed the 2-add-CREATE-INVISIBLE-new branch from eb99a47 to be9784b Compare

July 21, 2022 14:33

Contributor Author

wenyihu6 commented Jul 21, 2022

pkg/sql/opt/testutils/testcat/create_table.go line 708 at r3 (raw file):

Previously, knz (kena) wrote…

nit: panic(unimplemented.New(...))

Done.

Contributor Author

wenyihu6 commented Jul 21, 2022

pkg/sql/sem/tree/create.go line 235 at r3 (raw file):

Previously, knz (kena) wrote…

Rename this to NotVisible. Your future self will thank you.

Done. I'm sure I will.

Contributor Author

wenyihu6 commented Jul 21, 2022

pkg/sql/sem/tree/create.go line 985 at r3 (raw file):

Previously, knz (kena) wrote…

ditto

Done.

wenyihu6 force-pushed the 2-add-CREATE-INVISIBLE-new branch 2 times, most recently from b803557 to 30e30a8 Compare

July 21, 2022 15:11

wenyihu6 requested review from michae2 and mgartner

July 21, 2022 17:19

wenyihu6 force-pushed the 2-add-CREATE-INVISIBLE-new branch 5 times, most recently from d1c0928 to 6c341aa Compare

July 22, 2022 11:07

wenyihu6 added a commit to wenyihu6/cockroach that referenced this pull request


          sql: add ALTER INDEX … NOT VISIBLE to parser

be236f9

This commit adds parsing support for ALTER INDEX … [VISIBLE | NOT VISIBLE].
Executing the command returns an `unimplemented error`.

Assists: cockroachdb#72576

See also: cockroachdb#84783

Release note (sql change): Parser now supports altering an index to visible or
not visible. But no implementation has done yet, and executing it returns an
“unimplemented” error immediately.

wenyihu6 added a commit to wenyihu6/cockroach that referenced this pull request


          sql: support ALTER INDEX … NOT VISIBLE

144073e

This commit adds the actual execution for ALTER INDEX … [VISIBLE | NOT VISIBLE].

Assists: cockroachdb#72576

See also: cockroachdb#84783

Release note (sql change): Altering an index to visible or not visible using
ALTER INDEX … VISIBLE | NOT VISIBLE is now supported.

wenyihu6 added a commit to wenyihu6/cockroach that referenced this pull request


          sql: add ALTER INDEX … NOT VISIBLE to parser

d2eb196

This commit adds parsing support for ALTER INDEX … [VISIBLE | NOT VISIBLE].
Executing the command returns an `unimplemented error`.

Assists: cockroachdb#72576

See also: cockroachdb#84783

Release note (sql change): Parser now supports altering an index to visible or
not visible. But no implementation has done yet, and executing it returns an
“unimplemented” error immediately.

wenyihu6 added a commit to wenyihu6/cockroach that referenced this pull request


          sql: support ALTER INDEX … NOT VISIBLE

5a57f90

This commit adds the actual execution for ALTER INDEX … [VISIBLE | NOT VISIBLE].

Assists: cockroachdb#72576

See also: cockroachdb#84783

Release note (sql change): Altering an index to visible or not visible using
ALTER INDEX … VISIBLE | NOT VISIBLE is now supported.

wenyihu6 added a commit to wenyihu6/cockroach that referenced this pull request


          sql: add ALTER INDEX … NOT VISIBLE to parser

e624798

This commit adds parsing support for ALTER INDEX … [VISIBLE | NOT VISIBLE].
Executing the command returns an `unimplemented error`.

Assists: cockroachdb#72576

See also: cockroachdb#84783

Release note (sql change): Parser now supports altering an index to visible or
not visible. But no implementation has done yet, and executing it returns an
“unimplemented” error immediately.

# Conflicts:
#	pkg/sql/sem/tree/stmt.go

wenyihu6 deleted the 2-add-CREATE-INVISIBLE-new branch

September 1, 2022 20:48

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet