sql: permit user-defined hidden columns

[jordanlewis]

SQL supports hidden columns: columns that are only included in a projection if explicitly requested. An example of this is the rowid column that is created on tables without explicitly primary keys.

Currently, there is no way to get a hidden column on a user-defined table: it's only possible if you manually set the hidden boolean in the table descriptor, by editing the CockroachDB source code.

It could conceivably be useful for users to define hidden columns on their own tables. This issue tracks adding that functionality. It should be pretty straightforward, and reasonably suitable for a new contributor.

• Add an NOT VISIBLE keyword to pkg/sql/parser/sql.y as an optional modifier to columns for creation and modification. e.g. CREATE TABLE a (a INT NOT VISIBLE), ALTER TABLE a ALTER COLUMN a SET [NOT] VISIBLE sql: support NOT VISIBLE in CREATE TABLE for user-defined hidden columns #58923
• Add a NOT VISIBLE attribute to ColumnTableDef in pkg/sql/sem/tree/create.go sql: support NOT VISIBLE in CREATE TABLE for user-defined hidden columns #58923
• Update MakeColumnDefDescs in pkg/sql/catalog/tabledesc/table.go to set Hidden on the column descriptor if the attribute above is set sql: support NOT VISIBLE in CREATE TABLE for user-defined hidden columns #58923
• Add AlterTableSetHidden AST nodes to pkg/sql/sem/tree/alter_table.go
• Add cases for those AST nodes in pkg/sql/alter_table.go
• Add logic tests that verify that hidden columns work as expected. A good place for these would be in pkg/sql/logictest/testdata/logic_test/hidden. sql: support NOT VISIBLE in CREATE TABLE for user-defined hidden columns #58923

[jasobrown]

I'd like to take a stab at implementing this, if that's cool.

[jordanlewis]

You're more than welcome to, Jason! Thanks.

[rohany]

There might be some gotchas that we'll find out along the way. I think there might be some assertions lurking around that rowid is the only hidden column.

[jordanlewis]

I don't think that's true - didn't you just add the crdb_internal_mvcc_timestamp column to every table that's marked hidden too? :)

[rohany]

No, that was only within the optimizer's representation of tables.

[jordanlewis]

Hmm, ok. I think the hash sharded index columns are also marked hidden though, unless I'm missing something there too (makeShardColumnDesc)

[knz]

There was a draft implementation here: #26644

[knz]

That PR incidentally makes the case that it's needed to implement certain pg_catalog tables that also have hidden columns.

Also the approach at the top of this issue has a flaw. Look at the linked PR:

Note: the choice for the syntax "NOT VISIBLE" as opposed to "HIDDEN"
is mandated by the fact that the first word of an attribute must be a
reserved keyword, and adding reserved keywords is disruptive. This
way, the patch is properly backward-compatible.

[Heoric]

Which method should be used ?

ALTER TABLE a ALTER COLUMN a [SET|DROP] NOT VISIBLE
or
ALTER TABLE a ALTER COLUMN a SET [NOT] VISIBLE

[knz]

I changed the description of the issue to explain that the syntax must use "NOT VISIBLE".
We cannot use a keyword "HIDDEN" in the proposed way, because of grammar restrictions (see #26644 for details).

[knz]

ALTER TABLE a ALTER COLUMN a SET [NOT] VISIBLE

This is the correct one.

[knz]

Note: this feature is now discussed as well in this RFC: #58440

[knz]

PR #58923 is implementing the CREATE part of this, but does not implement ALTER SET [NOT] VISIBLE. @lucy-zhang requested that we delay the ALTER changes until after the schema change work has moved forward.

[thoszhang]

I don't think we should necessarily wait for the schema change work to proceed. I just thought it would be easier to review in a separate commit. Do we need ALTER SET VISIBLE for the multiregion work? I have no objections to implementing it.

[rafiss]

PR #63052 implements ALTER SET [NOT] VISIBLE