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: implement ALTER TABLE ... ALTER COLUMN SET [VISIBLE|NOT VISIBLE] #62892

Closed
ajstorm opened this issue Mar 31, 2021 · 8 comments
Closed

sql: implement ALTER TABLE ... ALTER COLUMN SET [VISIBLE|NOT VISIBLE] #62892

ajstorm opened this issue Mar 31, 2021 · 8 comments
Labels
A-multiregion Related to multi-region C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@ajstorm
Copy link
Collaborator

ajstorm commented Mar 31, 2021

In #58923, we added the ability to set a column as VISIBLE and NOT VISIBLE.

We now need the ability to have ALTER COLUMN <col> SET [VISIBLE|NOT VISIBLE].

The changes to do this are as follows (you can follow SET DEFAULT as a guideline for what to write)

  • Add a type AlterTableSetVisible struct { Visible bool } struct to tree/alter_table.go, similar to AlterTableSetDefault.
  • Add the syntax to sql.y.
  • Add the logic to rewrite the column descriptor to change the value of Hidden in alter_table.go
  • Add logic tests in logic_test/alter_table
  • Submit your PR!

Allow for ALTER COLUMN <column_name> SET NOT VISIBLE. When implemented, this statement will allow users to make their columns invisible. This functionality is required if customers want to build their own REGIONAL BY ROW tables (using REGIONAL BY ROW AS) and have them look identically to how they'd be created by the system.

@ajstorm ajstorm added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) A-multiregion Related to multi-region labels Mar 31, 2021
@otan otan changed the title sql: Allow for ALTER COLUMN SET NOT VISIBLE to make a column ... disappear sql: implement ALTER COLUMN SET [VISIBLE|NOT VISIBLE] Mar 31, 2021
@otan otan changed the title sql: implement ALTER COLUMN SET [VISIBLE|NOT VISIBLE] sql: implement ALTER TABLE ... ALTER COLUMN SET [VISIBLE|NOT VISIBLE] Mar 31, 2021
@kurokochin
Copy link
Contributor

Hi, I'm new to cockroachdb, but I'm willing to give it a try, can I work on it?

craig bot pushed a commit that referenced this issue Apr 8, 2021
63052: sql: support alter column SET VISIBLE and NOT VISIBLE to unhide or hide column r=otan a=kurokochin

Addresses #62892 

Co-authored-by: kurokochin <[email protected]>
@rafiss
Copy link
Collaborator

rafiss commented Apr 9, 2021

just checking: does #63052 close this?

@rafiss
Copy link
Collaborator

rafiss commented Apr 14, 2021

@ajstorm @otan does this need to be in 21.1?

@otan
Copy link
Contributor

otan commented Apr 15, 2021

I think we're back porting this to 21.1.1

@ajstorm
Copy link
Collaborator Author

ajstorm commented Apr 15, 2021 via email

@rafiss rafiss closed this as completed Apr 15, 2021
@rafiss
Copy link
Collaborator

rafiss commented Apr 15, 2021

@ajstorm The convention is to use the backport-21.1.x label (and so on) on the PR. I added that to #63052

However, that's not super easy to track, (as I posited in Slack https://cockroachlabs.slack.com/archives/C0HM2DZ34/p1618516063393000) so I'm going to try to make https://backboard.crdb.dev/ understand these labels in some way

@rafiss rafiss reopened this Apr 15, 2021
@otan
Copy link
Contributor

otan commented Apr 19, 2021

This just needs backporting now.

@mgartner
Copy link
Collaborator

This was fixed by #63052. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-multiregion Related to multi-region C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
None yet
Development

No branches or pull requests

5 participants