-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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: added CREATE … NOT VISIBLE to parser #83471
Conversation
> We already use the syntax "NOT VISIBLE" for a similar feature elsewhere. Could we avoid introducing a new keyword and instead reuse the existing syntax, for consistency in the UX?
> […](#)
> On Mon, Jun 27, 2022 at 5:44 PM Wenyi Hu ***@***.***> wrote: This PR takes the second step for the invisible index feature. The previous PR, #83388 <#83388>, should be merged before this pr is merged. The first two commits of this pr comes from this previous pr. This PR added the invisible index feature for CREATE INDEX … and CREATE TABLE (INDEX …). The first commit added INVISIBLE to the parser. The second commit changed the optimizer to ignore any invisible indexes unless it is selected with index hinting. Note that this pr does not include adding invisible indexes with UNIQUE CONSTRAINT or UNIQUE INDEX yet. Release note (sql change): The invisible index feature has been introduced now, but they only work with CREATE INDEX …, CREATE TABLE (INDEX). This will be changed soon. See also: #83388 <#83388> There will be more PR coming up to add the SQL statements ALTER INDEX … INVISIBLE, CREATE UNIQUE WITH INVISIBLE INDEX, ADD UNIQUE CONSTRAINT WITH INVISIBLE INDEX. (TODO (wenyihu6)): link related pr later Fixes: #72576 <#72576> Note that this issue has not been resolved yet. ------------------------------ You can view, comment on, or merge this pull request online at: #83471 Commit Summary - [4287fe7](https://github.com/cockroachdb/cockroach/commit/4287fe7ad72822710a41797097a38e14a4a6452c) <[4287fe7](https://github.com//pull/83471/commits/4287fe7ad72822710a41797097a38e14a4a6452c)> 1-sql: added invisible to index descriptor - [b659287](https://github.com/cockroachdb/cockroach/commit/b659287390d5e01b9db1ed045fbe5bf1a55f3169) <[b659287](https://github.com//pull/83471/commits/b659287390d5e01b9db1ed045fbe5bf1a55f3169)> 2-sql: added is_invisible to crdb_internal and SHOW INDEX - [178826e](https://github.com/cockroachdb/cockroach/commit/178826ee14f8a7113311f20a9886cd8354a89203) <[178826e](https://github.com//pull/83471/commits/178826ee14f8a7113311f20a9886cd8354a89203)> 1-sql: added INVISIBLE to create index and create table - [cc8d2ba](https://github.com/cockroachdb/cockroach/commit/cc8d2bad4f7f04b6bd508c0d5cd5bff262e55619) <[cc8d2ba](https://github.com//pull/83471/commits/cc8d2bad4f7f04b6bd508c0d5cd5bff262e55619)> 2-sql: add invisible index feature to optimizer File Changes (66 files ) - *M* docs/generated/sql/bnf/create_index_stmt.bnf (4) - *M* docs/generated/sql/bnf/create_inverted_index_stmt.bnf (8) - *M* docs/generated/sql/bnf/index_def.bnf (20) - *M* docs/generated/sql/bnf/stmt_block.bnf (20) - *M* docs/generated/sql/bnf/table_constraint.bnf (16) - *M* pkg/ccl/logictestccl/testdata/logic_test/crdb_internal_tenant (4) - *M* pkg/sql/alter_primary_key.go (1) - *M* pkg/sql/alter_table.go (1) - *M* pkg/sql/catalog/catformat/index.go (4) - *M* pkg/sql/catalog/descpb/index.go (5) - *M* pkg/sql/catalog/descpb/structured.proto (10) - *M* pkg/sql/catalog/systemschema/system.go (1) - *M* pkg/sql/catalog/table_elements.go (1) - *M* pkg/sql/catalog/tabledesc/index.go (5) - *M* pkg/sql/catalog/tabledesc/structured.go (1) - *M* pkg/sql/catalog/tabledesc/table.go (3) - *M* pkg/sql/catalog/tabledesc/validate.go (4) - *M* pkg/sql/catalog/tabledesc/validate_test.go (31) - *M* pkg/sql/crdb_internal.go (2) - *M* pkg/sql/create_index.go (1) - *M* pkg/sql/create_sequence.go (1) - *M* pkg/sql/create_table.go (11) - *M* pkg/sql/delegate/show_database_indexes.go (24) - *M* pkg/sql/delegate/show_table.go (12) - *M* pkg/sql/descriptor_mutation_test.go (10) - *M* pkg/sql/importer/read_import_pgdump.go (1) - *M* pkg/sql/logictest/testdata/logic_test/alter_primary_key (76) - *M* pkg/sql/logictest/testdata/logic_test/alter_table (48) - *M* pkg/sql/logictest/testdata/logic_test/crdb_internal (4) - *M* pkg/sql/logictest/testdata/logic_test/create_index (64) - *M* pkg/sql/logictest/testdata/logic_test/create_statements (2) - *M* pkg/sql/logictest/testdata/logic_test/create_table (3) - *M* pkg/sql/logictest/testdata/logic_test/drop_index (112) - *M* pkg/sql/logictest/testdata/logic_test/fk (64) - *M* pkg/sql/logictest/testdata/logic_test/information_schema (4) - *M* pkg/sql/logictest/testdata/logic_test/new_schema_changer (96) - *M* pkg/sql/logictest/testdata/logic_test/pg_catalog (4) - *M* pkg/sql/logictest/testdata/logic_test/rename_column (20) - *M* pkg/sql/logictest/testdata/logic_test/rename_index (74) - *M* pkg/sql/logictest/testdata/logic_test/show_indexes (72) - *M* pkg/sql/logictest/testdata/logic_test/show_source (74) - *M* pkg/sql/logictest/testdata/logic_test/storing (28) - *M* pkg/sql/logictest/testdata/logic_test/table (82) - *M* pkg/sql/opt/cat/index.go (3) - *M* pkg/sql/opt/cat/utils.go (8) - *M* pkg/sql/opt/exec/execbuilder/testdata/explain (19) - *M* pkg/sql/opt/exec/execbuilder/testdata/select (28) - *M* pkg/sql/opt/exec/execbuilder/testdata/select_index (14) - *M* pkg/sql/opt/exec/execbuilder/testdata/show_trace (16) - *M* pkg/sql/opt/indexrec/hypothetical_index.go (8) - *A* pkg/sql/opt/testutils/opttester/testdata/invisible-index (406) - *M* pkg/sql/opt/testutils/testcat/create_index.go (1) - *M* pkg/sql/opt/testutils/testcat/create_table.go (19) - *M* pkg/sql/opt/testutils/testcat/test_catalog.go (8) - *M* pkg/sql/opt/xform/scan_index_iter.go (16) - *M* pkg/sql/opt_catalog.go (10) - *M* pkg/sql/parser/sql.y (37) - *M* pkg/sql/parser/testdata/create_index (53) - *M* pkg/sql/parser/testdata/create_table (74) - *M* pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table_add_column.go (1) - *M* pkg/sql/schemachanger/scbuild/internal/scbuildstmt/create_index.go (1) - *M* pkg/sql/schemachanger/scdecomp/decomp.go (1) - *M* pkg/sql/schemachanger/scexec/scmutationexec/index.go (1) - *M* pkg/sql/schemachanger/scpb/elements.proto (4) - *M* pkg/sql/sem/tree/create.go (8) - *M* pkg/sql/sem/tree/pretty.go (13) Patch Links: - https://github.com//pull/83471.patch - https://github.com//pull/83471.diff — Reply to this email directly, view it on GitHub <#83471>, or unsubscribe . You are receiving this because your review was requested.Message ID: ***@***.***>
Thanks for taking a look at this PR! I think the similar feature you are |
> Also, nit: in this PR and the other one #83388, the text "see also" and the other things that follow should be placed **above** the release note. This is because the release note extraction considers _all_ the text after "Release note" up to the end of the commit message to be part of the release note.
> […](#)
> On Mon, Jun 27, 2022 at 5:44 PM Wenyi Hu ***@***.***> wrote: This PR takes the second step for the invisible index feature. The previous PR, #83388 <#83388>, should be merged before this pr is merged. The first two commits of this pr comes from this previous pr. This PR added the invisible index feature for CREATE INDEX … and CREATE TABLE (INDEX …). The first commit added INVISIBLE to the parser. The second commit changed the optimizer to ignore any invisible indexes unless it is selected with index hinting. Note that this pr does not include adding invisible indexes with UNIQUE CONSTRAINT or UNIQUE INDEX yet. Release note (sql change): The invisible index feature has been introduced now, but they only work with CREATE INDEX …, CREATE TABLE (INDEX). This will be changed soon. See also: #83388 <#83388> There will be more PR coming up to add the SQL statements ALTER INDEX … INVISIBLE, CREATE UNIQUE WITH INVISIBLE INDEX, ADD UNIQUE CONSTRAINT WITH INVISIBLE INDEX. (TODO (wenyihu6)): link related pr later Fixes: #72576 <#72576> Note that this issue has not been resolved yet. ------------------------------ You can view, comment on, or merge this pull request online at: #83471 Commit Summary - [4287fe7](https://github.com/cockroachdb/cockroach/commit/4287fe7ad72822710a41797097a38e14a4a6452c) <[4287fe7](https://github.com//pull/83471/commits/4287fe7ad72822710a41797097a38e14a4a6452c)> 1-sql: added invisible to index descriptor - [b659287](https://github.com/cockroachdb/cockroach/commit/b659287390d5e01b9db1ed045fbe5bf1a55f3169) <[b659287](https://github.com//pull/83471/commits/b659287390d5e01b9db1ed045fbe5bf1a55f3169)> 2-sql: added is_invisible to crdb_internal and SHOW INDEX - [178826e](https://github.com/cockroachdb/cockroach/commit/178826ee14f8a7113311f20a9886cd8354a89203) <[178826e](https://github.com//pull/83471/commits/178826ee14f8a7113311f20a9886cd8354a89203)> 1-sql: added INVISIBLE to create index and create table - [cc8d2ba](https://github.com/cockroachdb/cockroach/commit/cc8d2bad4f7f04b6bd508c0d5cd5bff262e55619) <[cc8d2ba](https://github.com//pull/83471/commits/cc8d2bad4f7f04b6bd508c0d5cd5bff262e55619)> 2-sql: add invisible index feature to optimizer File Changes (66 files ) - *M* docs/generated/sql/bnf/create_index_stmt.bnf (4) - *M* docs/generated/sql/bnf/create_inverted_index_stmt.bnf (8) - *M* docs/generated/sql/bnf/index_def.bnf (20) - *M* docs/generated/sql/bnf/stmt_block.bnf (20) - *M* docs/generated/sql/bnf/table_constraint.bnf (16) - *M* pkg/ccl/logictestccl/testdata/logic_test/crdb_internal_tenant (4) - *M* pkg/sql/alter_primary_key.go (1) - *M* pkg/sql/alter_table.go (1) - *M* pkg/sql/catalog/catformat/index.go (4) - *M* pkg/sql/catalog/descpb/index.go (5) - *M* pkg/sql/catalog/descpb/structured.proto (10) - *M* pkg/sql/catalog/systemschema/system.go (1) - *M* pkg/sql/catalog/table_elements.go (1) - *M* pkg/sql/catalog/tabledesc/index.go (5) - *M* pkg/sql/catalog/tabledesc/structured.go (1) - *M* pkg/sql/catalog/tabledesc/table.go (3) - *M* pkg/sql/catalog/tabledesc/validate.go (4) - *M* pkg/sql/catalog/tabledesc/validate_test.go (31) - *M* pkg/sql/crdb_internal.go (2) - *M* pkg/sql/create_index.go (1) - *M* pkg/sql/create_sequence.go (1) - *M* pkg/sql/create_table.go (11) - *M* pkg/sql/delegate/show_database_indexes.go (24) - *M* pkg/sql/delegate/show_table.go (12) - *M* pkg/sql/descriptor_mutation_test.go (10) - *M* pkg/sql/importer/read_import_pgdump.go (1) - *M* pkg/sql/logictest/testdata/logic_test/alter_primary_key (76) - *M* pkg/sql/logictest/testdata/logic_test/alter_table (48) - *M* pkg/sql/logictest/testdata/logic_test/crdb_internal (4) - *M* pkg/sql/logictest/testdata/logic_test/create_index (64) - *M* pkg/sql/logictest/testdata/logic_test/create_statements (2) - *M* pkg/sql/logictest/testdata/logic_test/create_table (3) - *M* pkg/sql/logictest/testdata/logic_test/drop_index (112) - *M* pkg/sql/logictest/testdata/logic_test/fk (64) - *M* pkg/sql/logictest/testdata/logic_test/information_schema (4) - *M* pkg/sql/logictest/testdata/logic_test/new_schema_changer (96) - *M* pkg/sql/logictest/testdata/logic_test/pg_catalog (4) - *M* pkg/sql/logictest/testdata/logic_test/rename_column (20) - *M* pkg/sql/logictest/testdata/logic_test/rename_index (74) - *M* pkg/sql/logictest/testdata/logic_test/show_indexes (72) - *M* pkg/sql/logictest/testdata/logic_test/show_source (74) - *M* pkg/sql/logictest/testdata/logic_test/storing (28) - *M* pkg/sql/logictest/testdata/logic_test/table (82) - *M* pkg/sql/opt/cat/index.go (3) - *M* pkg/sql/opt/cat/utils.go (8) - *M* pkg/sql/opt/exec/execbuilder/testdata/explain (19) - *M* pkg/sql/opt/exec/execbuilder/testdata/select (28) - *M* pkg/sql/opt/exec/execbuilder/testdata/select_index (14) - *M* pkg/sql/opt/exec/execbuilder/testdata/show_trace (16) - *M* pkg/sql/opt/indexrec/hypothetical_index.go (8) - *A* pkg/sql/opt/testutils/opttester/testdata/invisible-index (406) - *M* pkg/sql/opt/testutils/testcat/create_index.go (1) - *M* pkg/sql/opt/testutils/testcat/create_table.go (19) - *M* pkg/sql/opt/testutils/testcat/test_catalog.go (8) - *M* pkg/sql/opt/xform/scan_index_iter.go (16) - *M* pkg/sql/opt_catalog.go (10) - *M* pkg/sql/parser/sql.y (37) - *M* pkg/sql/parser/testdata/create_index (53) - *M* pkg/sql/parser/testdata/create_table (74) - *M* pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table_add_column.go (1) - *M* pkg/sql/schemachanger/scbuild/internal/scbuildstmt/create_index.go (1) - *M* pkg/sql/schemachanger/scdecomp/decomp.go (1) - *M* pkg/sql/schemachanger/scexec/scmutationexec/index.go (1) - *M* pkg/sql/schemachanger/scpb/elements.proto (4) - *M* pkg/sql/sem/tree/create.go (8) - *M* pkg/sql/sem/tree/pretty.go (13) Patch Links: - https://github.com//pull/83471.patch - https://github.com//pull/83471.diff — Reply to this email directly, view it on GitHub <#83471>, or unsubscribe . You are receiving this because your review was requested.Message ID: ***@***.***>
Done. |
844ec0d
to
032493e
Compare
This PR takes the first step for the invisible index feature. The first commit added a boolean field `Hidden` to the struct `IndexDescriptor`. Since primary indexes cannot be invisible, this commit also added a check in `pkg/sql/catalog/tabledesc/validate.go` for that. The second commit added a new column `is_hidden` to `crdb_internal.table_indexes` and also to the output of following SQL statements: ``` SHOW INDEX FROM (table_name) SHOW INDEXES FROM(table_name) SHOW KEYS FROM (table_name) SHOW INDEX FROM DATABASE(database_name) SHOW INDEXES FROM DATABASE (database_name) SHOW KEYS FROM DATABASE (database_name) ``` Since the invisible index feature has not been introduced yet, all indexes created should be visible. It is expected for all test cases to output `false` for all `is_hidden` columns. See also: There will be more PR coming up to add the SQL statements `CREATE INDEX … NOT VISIBLE`, `CREATE TABLE … (INDEX NOT VISIBLE)`, `ALTER INDEX … NOT VISIBLE`. Next PR for invisible index feature: cockroachdb#83471 TODO (wenyihu6): link more related pr later on Fixes: cockroachdb#72576 Note that this issue has not been resolved yet, and this pr only takes the first step. Release note (sql change): A new column `is_hidden` has been added to the table `crdb_internal.table_indexes` and to the output of SQL statements related to `SHOW INDEX`, `SHOW INDEXES`, and `SHOW KEYS`. Since the invisible index feature has not been introduced yet, the output for this column will always be false for now. This will be changed soon.
This PR added a new field `NotVisible` to the struct `IndexDescriptor`. Since primary indexes cannot be not visible, it added another test in `pkg/sql/catalog/tabledesc/validate.go`. Since the invisible index feature has not been introduced yet, all indexes created should be visible. See also: cockroachdb#83471 Assists: cockroachdb#72576 Release note: none
This PR added a new field `NotVisible` to the struct `IndexDescriptor`. Since primary indexes cannot be not visible, it added another test in `pkg/sql/catalog/tabledesc/validate.go`. Since the invisible index feature has not been introduced yet, all indexes created should be visible. See also: cockroachdb#83471 Assists: cockroachdb#72576 Release note: none
This PR adds a new column `is_visible` to `crdb_internal.table_indexes` and `information_schema.statistics`. It also adds a new column `visible` to the output of following SQL statements: ``` SHOW INDEX FROM (table_name) SHOW INDEXES FROM (table_name) SHOW KEYS FROM (table_name) SHOW INDEX FROM DATABASE (database_name) SHOW INDEXES FROM DATABASE (database_name) SHOW KEYS FROM DATABASE (database_name) ``` Since the not visible index feature has not been introduced yet, it is expected for all test cases to output `true` for all `is_visible` or `visible` columns. See also: cockroachdb#83471 Assists: cockroachdb#72576 Release note (sql change): A new column `is_visible` has been added to the table `crdb_internal.table_indexes` and `information_schema.statistics`. A new column `visible` has also been added to the output of `SHOW INDEX`, `SHOW INDEXES`, and `SHOW KEYS`. The `is_visible` or `visible` column indicates whether the index is visible to the optimizer.
This PR takes the second step for the invisible index feature. The previous PR, cockroachdb#83388, should be merged before this PR is merged. The first commit of this pr comes from the previous pr. This PR added parsing support for CREATE INDEX … NOT VISIBLE and CREATE TABLE (... INDEX() NOT VISIBLE). Note that this PR does not add any logic to the optimizer, and executing it returns an “unimplemented” error immediately. Assists: cockroachdb#72576 See also: cockroachdb#83388 Release note: None
38329c5
to
5110ca2
Compare
Opened a new PR for a cleaner thread without syntax discussion #84783. |
This PR adds a new column `is_visible` to `crdb_internal.table_indexes` and `information_schema.statistics`. It also adds a new column `visible` to the output of following SQL statements: ``` SHOW INDEX FROM (table_name) SHOW INDEXES FROM (table_name) SHOW KEYS FROM (table_name) SHOW INDEX FROM DATABASE (database_name) SHOW INDEXES FROM DATABASE (database_name) SHOW KEYS FROM DATABASE (database_name) ``` Since the not visible index feature has not been introduced yet, it is expected for all test cases to output `true` for all `is_visible` or `visible` columns. See also: cockroachdb#83471 Assists: cockroachdb#72576 Release note (sql change): A new column `is_visible` has been added to the table `crdb_internal.table_indexes` and `information_schema.statistics`. A new column `visible` has also been added to the output of `SHOW INDEX`, `SHOW INDEXES`, and `SHOW KEYS`. The `is_visible` or `visible` column indicates whether the index is visible to the optimizer.
This PR adds a new column `is_visible` to `crdb_internal.table_indexes` and `information_schema.statistics`. It also adds a new column `visible` to the output of following SQL statements: ``` SHOW INDEX FROM (table_name) SHOW INDEXES FROM (table_name) SHOW KEYS FROM (table_name) SHOW INDEX FROM DATABASE (database_name) SHOW INDEXES FROM DATABASE (database_name) SHOW KEYS FROM DATABASE (database_name) ``` Since the not visible index feature has not been introduced yet, it is expected for all test cases to output `true` for all `is_visible` or `visible` columns. See also: cockroachdb#83471 Assists: cockroachdb#72576 Release note (sql change): A new column `is_visible` has been added to the table `crdb_internal.table_indexes` and `information_schema.statistics`. A new column `visible` has also been added to the output of `SHOW INDEX`, `SHOW INDEXES`, and `SHOW KEYS`. The `is_visible` or `visible` column indicates whether the index is visible to the optimizer.
This PR adds a new column `is_visible` to `crdb_internal.table_indexes` and `information_schema.statistics`. It also adds a new column `visible` to the output of following SQL statements: ``` SHOW INDEX FROM (table_name) SHOW INDEXES FROM (table_name) SHOW KEYS FROM (table_name) SHOW INDEX FROM DATABASE (database_name) SHOW INDEXES FROM DATABASE (database_name) SHOW KEYS FROM DATABASE (database_name) ``` Since the not visible index feature has not been introduced yet, it is expected for all test cases to output `true` for all `is_visible` or `visible` columns. See also: cockroachdb#83471 Assists: cockroachdb#72576 Release note (sql change): A new column `is_visible` has been added to the table `crdb_internal.table_indexes` and `information_schema.statistics`. A new column `visible` has also been added to the output of `SHOW INDEX`, `SHOW INDEXES`, and `SHOW KEYS`. The `is_visible` or `visible` column indicates whether the index is visible to the optimizer.
This commit adds a new column `is_visible` to `crdb_internal.table_indexes` and `information_schema.statistics`. It also adds a new column `visible` to the output of following SQL statements: ``` SHOW INDEX FROM (table_name) SHOW INDEXES FROM (table_name) SHOW KEYS FROM (table_name) SHOW INDEX FROM DATABASE (database_name) SHOW INDEXES FROM DATABASE (database_name) SHOW KEYS FROM DATABASE (database_name) ``` Since the not visible index feature has not been introduced yet, it is expected for all test cases to output `true` for all `is_visible` or `visible` columns. See also: cockroachdb#83471 Assists: cockroachdb#72576 Release note (sql change): A new column `is_visible` has been added to the table `crdb_internal.table_indexes` and `information_schema.statistics`. A new column `visible` has also been added to the output of `SHOW INDEX`, `SHOW INDEXES`, and `SHOW KEYS`. The `is_visible` or `visible` column indicates whether the index is visible to the optimizer.
This PR adds parsing support for
CREATE INDEX … NOT VISIBLE
and
CREATE TABLE (... INDEX() NOT VISIBLE)
.This PR does not add any logic to the optimizer, and executing it returns an
“unimplemented” error immediately.
Assists: #72576
See also: #83388
Release note: None