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: initial support for virtual columns #57803

Merged
merged 1 commit into from
Dec 12, 2020

Conversation

RaduBerinde
Copy link
Member

@RaduBerinde RaduBerinde commented Dec 10, 2020

This change adds very basic support for virtual columns to the
non-test descriptors and catalog. This is gated behind an experimental
session setting.

Informs #57608.

Release note: None

@RaduBerinde RaduBerinde requested a review from a team as a code owner December 10, 2020 20:43
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@RaduBerinde RaduBerinde force-pushed the virtual-columns-real branch 2 times, most recently from c4c0459 to c5f97e7 Compare December 10, 2020 21:37
Copy link
Member Author

@RaduBerinde RaduBerinde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @ajwerner, @mgartner, and @rytaft)


pkg/sql/opt/exec/execbuilder/testdata/virtual_columns, line 18 at r1 (raw file):

 ├── tableoid oid [hidden] [system]
 ├── FAMILY fam_0_b (b)
 ├── FAMILY fam_1_a_v (a, v)

Hm, I think virtual column should not be part of any column family. I'll fix that.

@RaduBerinde
Copy link
Member Author

Added logic to disallow virtual columns from being part of column families; and added validation logic for the Virtual field.

Copy link
Contributor

@ajwerner ajwerner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was straightforward. Is there anything that needs to be done to use these in indexes, constraints, etc? If they would work, might be nice to add some testing, otherwise, might be nice to put up some gating.

:lgtm:

Reviewed 9 of 13 files at r1, 8 of 9 files at r2.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @ajwerner, @mgartner, and @rytaft)

@RaduBerinde
Copy link
Member Author

Yeah, quite a bit needs to be done, especially for indexes. I'm keeping a checklist in #57608. For now, we just have this one big gate. We'll consider gating more specific things when we get further along.

This change adds very basic support for virtual columns to the
non-test descriptors and catalog. This is gated behind an experimental
session setting.

Release note: None
@RaduBerinde
Copy link
Member Author

Fixed and added some ValidateTableDescriptor tests.

@RaduBerinde
Copy link
Member Author

bors r+

@craig
Copy link
Contributor

craig bot commented Dec 12, 2020

Build succeeded:

@craig craig bot merged commit 960b4cf into cockroachdb:master Dec 12, 2020
@RaduBerinde RaduBerinde deleted the virtual-columns-real branch December 16, 2020 22:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants