cockroachdb / cockroach

CockroachDB - the open source, cloud-native distributed SQL database.
https://www.cockroachlabs.com
Other
29.59k stars 3.71k forks source link

opt: normalize constraints #27018

Open justinj opened 6 years ago

justinj commented 6 years ago

Extracted from comments on #26962:

We should normalize multi-column constraints so it's easy to manipulate and extract things like constant columns, and also specify the form in which code creating constraints should create them. I think in the long term code should always be creating the most precise constraints they can, and then we rely on the ConstraintSet code to represent them in a good way.

From @andy-kimball:

I guess what bothers me here is that we're not normalizing in the constraint code, but are instead adding a bunch of redundant constraints in the builder code and then expecting constraint.Set to deal with them. It seems inelegant and inefficient. I'd much prefer to be normalizing in constraint.Set according to whatever rules we decide upon. If we want to maintain redundant constraints (and there are good reasons to do so), then we should enforce that in constraint.Set. That way, every caller that uses Intersect/Union will get the right behavior, rather than being expected to add the right constraint combos themselves. I could imagine rules like:

Multi-column constraints will always have exact prefixes factored out (according to ExactPrefix method):

/y/m/d: [/2018/10/2 - /2018/11/2] => /y: [/2018 - /2018] /m/d: [/10/2 - /11/2]

/y/m/d: [/2018/10/2 - /2018/10/5] [/2018/10/15 - /2018/10/20] => /y: [/2018 - /2018] /m: [/10 - /10] /d: [/2 - /5] [/15 - /20]

Every column appearing in a multi-column constraint will also have a single-column constraint, assuming its single-col version is not unconstrained:

/y/m/d: [/2018/10/2 - /2018/11/2] => /y: [/2018 - /2018] /m/d: [/10/2 - /11/2] /m: [/10 - /11]

/y/m/d: [/2018/10/2 - /2018/10/5] [/2020/10/12 - /2020/10/15] => /y/m/d: [/2018/10/2 - /2018/10/5] [/2020/10/12 - /2020/10/15] /y: [/2018 - /2018] [/2020 - /2020] /m: [/10 - /10] /d: [/2 - /5] [/12 - /15]

I think we should go ahead with this PR, but I wanted to record my reservations, in the hopes that we can tighten this up in the future.

cc @RaduBerinde @rytaft @madhavsuresh

Jira issue: CRDB-4974

github-actions[bot] commented 3 years ago

We have marked this issue as stale because it has been inactive for 18 months. If this issue is still relevant, removing the stale label or adding a comment will keep it active. Otherwise, we'll close it in 5 days to keep the issue queue tidy. Thank you for your contribution to CockroachDB!

github-actions[bot] commented 10 months ago

We have marked this issue as stale because it has been inactive for 18 months. If this issue is still relevant, removing the stale label or adding a comment will keep it active. Otherwise, we'll close it in 10 days to keep the issue queue tidy. Thank you for your contribution to CockroachDB!