The Wayback Machine - https://web.archive.org/web/20200902025902/https://github.com/cockroachdb/cockroach/issues/53428
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

sql: permit user-defined hidden columns #53428

Open
jordanlewis opened this issue Aug 25, 2020 · 8 comments
Open

sql: permit user-defined hidden columns #53428

jordanlewis opened this issue Aug 25, 2020 · 8 comments

Comments

@jordanlewis
Copy link
Member

@jordanlewis jordanlewis commented Aug 25, 2020

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 HIDDEN keyword to pkg/sql/parser/sql.y as an optional modifier to columns for creation and modification. e.g. CREATE TABLE a (a INT HIDDEN), ALTER TABLE a ALTER COLUMN a [SET|DROP] HIDDEN
  • Add a HIDDEN attribute to ColumnTableDef in pkg/sql/sem/tree/create.go
  • Update MakeColumnDefDescs in pkg/sql/catalog/tabledesc/table.go to set Hidden on the column descriptor if the attribute above is set
  • 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.
@jordanlewis jordanlewis added this to Triage in SQL Features via automation Aug 25, 2020
@jasobrown
Copy link
Contributor

@jasobrown jasobrown commented Aug 25, 2020

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

@jordanlewis
Copy link
Member Author

@jordanlewis jordanlewis commented Aug 25, 2020

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

@rohany
Copy link
Contributor

@rohany rohany commented Aug 25, 2020

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
Copy link
Member Author

@jordanlewis jordanlewis commented Aug 25, 2020

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
Copy link
Contributor

@rohany rohany commented Aug 25, 2020

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

@jordanlewis
Copy link
Member Author

@jordanlewis jordanlewis commented Aug 25, 2020

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

@knz
Copy link
Member

@knz knz commented Aug 26, 2020

There was a draft implementation here: #26644

@knz
Copy link
Member

@knz knz commented Aug 26, 2020

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.

@rohany rohany moved this from Triage to 20.2 in SQL Features Aug 26, 2020
@rohany rohany moved this from 20.2 to Backlog in SQL Features Aug 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.