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

schemachanger: enable adding/dropping path of constraint name #90840

Conversation

Xiang-Gu
Copy link
Contributor

@Xiang-Gu Xiang-Gu commented Oct 28, 2022

Enable adding/dropping path of constraint name. It's considered
a simple dependent of constraint, so it transition directly between absent
and public.

Fixes #89665
Release note: None

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@Xiang-Gu Xiang-Gu force-pushed the enable_adding_and_dropping_of_constraint_name_element branch from 159e480 to 701b3eb Compare October 31, 2022 16:08
@Xiang-Gu Xiang-Gu force-pushed the enable_adding_and_dropping_of_constraint_name_element branch from 701b3eb to 124085d Compare November 14, 2022 16:47
@Xiang-Gu Xiang-Gu force-pushed the enable_adding_and_dropping_of_constraint_name_element branch from 124085d to 9c717b6 Compare November 14, 2022 23:17
@Xiang-Gu Xiang-Gu marked this pull request as ready for review November 14, 2022 23:18
@Xiang-Gu Xiang-Gu requested a review from a team November 14, 2022 23:18
craig bot pushed a commit that referenced this pull request Nov 15, 2022
91704: *: Improve constraints retrieval in table descriptor r=Xiang-Gu a=Xiang-Gu

This PR tries to improve how we retrieve constraints in a table descriptor. Previously,
it was mostly legacy code carries over from a while ago and nothing hasn't really
changed.
The main change is to introduce `catalog.Constraint` interface, similar to `catalog.Index`
and `catalog.Column`, as the preferred interface for constraint in this layer. Previously,
we would directly expose the underlying protobuf descriptor.

Commit 1 (easy): Rename `catalog.ConstraintToUpdate` to `catalog.Constraint`. 
It's good that we already have an interface that is suitable to be used for our effort.

Commit 2 (easy): Added methods in just renamed `catalog.Constraint` interface for
index-backed-constraints (i.e. PRIMARY KEY or UNIQUE);

Commit 3 (easy): Let `tabledesc.index` struct implement `catalog.Constraint` interface
as we will use it for index-backed-constraints.

Commit 4 (easy): Add a method in `catalog.Constraint` that gets validity of the constraint.

Commit 5 (moderate): Add logic (`ConstraintCache`) to pre-compute all constraints in a
table, categorized by type and validity, so that we can readily retrieve them later. This is the same
idea/technique used for managing index and columns (in `IndexCache` and `ColumnCache`).

Commit 6 (easy): Introduce the new, preferred methods in `catalog.TableDescriptor` to
retrieve constraints from a table.

Commit 7 (easy): Refactor signature of the existing `FindConstraintWithID` method to use
the newly added interface and retrieval methods.

Informs: #90840 
(this PR can unblock #90840)

Release note: None

91867: ui: change height of column selector r=maryliag a=maryliag

Previosuly, it was hard to identify there was more items on the columns selector, since the scrollbar is confugured by the user and might not show up right away (it will show once you hover with mouse and scroll).
This commit changes the height of the filter, making part of the next options to show up, hinting there is more options when scrolling.

Part Of #91763

Before
<img width="322" alt="Screen Shot 2022-11-14 at 2 59 39 PM" src="https://user-images.githubusercontent.com/1017486/201755400-1276e45b-62b8-44c0-a7ff-c337090ad94a.png">

After
<img width="308" alt="Screen Shot 2022-11-14 at 3 02 47 PM" src="https://user-images.githubusercontent.com/1017486/201755427-906e1c3b-e9fa-443b-9508-b2957b38d90b.png">


Release note (ui change): Change the height of column selector, so it can hint there are more options to be selected once scrolled.

Co-authored-by: Xiang Gu <[email protected]>
Co-authored-by: maryliag <[email protected]>
The old schema changer has the behavior of retaining the to-be-dropped
check constraint on the `Checks` slice. We will do the same in the
new schema changer.
ConstraintName element is deemed simple dependents of constraint and
hence has transition between PUBLIC <==> ABSENT. This commit thus
is primarily implementing the scop `SetConstraintName`.
@Xiang-Gu Xiang-Gu force-pushed the enable_adding_and_dropping_of_constraint_name_element branch from 9c717b6 to 65035fe Compare November 15, 2022 15:29
@Xiang-Gu
Copy link
Contributor Author

This is ready for a look!

@Xiang-Gu
Copy link
Contributor Author

closing as it's now part of #91153

@Xiang-Gu Xiang-Gu closed this Nov 29, 2022
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.

schemachanger: Enable adding/dropping path of check constraint and constraint names
2 participants