{"action":"opened","issue":{"url":"https://api.github.com/repos/cockroachdb/cockroach/issues/27018","repository_url":"https://api.github.com/repos/cockroachdb/cockroach","labels_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/27018/labels{/name}","comments_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/27018/comments","events_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/27018/events","html_url":"https://github.com/cockroachdb/cockroach/issues/27018","id":336288152,"node_id":"MDU6SXNzdWUzMzYyODgxNTI=","number":27018,"title":"opt: normalize constraints","user":{"login":"justinj","id":409075,"node_id":"MDQ6VXNlcjQwOTA3NQ==","avatar_url":"https://avatars2.githubusercontent.com/u/409075?v=4","gravatar_id":"","url":"https://api.github.com/users/justinj","html_url":"https://github.com/justinj","followers_url":"https://api.github.com/users/justinj/followers","following_url":"https://api.github.com/users/justinj/following{/other_user}","gists_url":"https://api.github.com/users/justinj/gists{/gist_id}","starred_url":"https://api.github.com/users/justinj/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/justinj/subscriptions","organizations_url":"https://api.github.com/users/justinj/orgs","repos_url":"https://api.github.com/users/justinj/repos","events_url":"https://api.github.com/users/justinj/events{/privacy}","received_events_url":"https://api.github.com/users/justinj/received_events","type":"User","site_admin":false},"labels":[{"id":843052634,"node_id":"MDU6TGFiZWw4NDMwNTI2MzQ=","url":"https://api.github.com/repos/cockroachdb/cockroach/labels/A-sql-planning","name":"A-sql-planning","color":"22aa88","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2018-06-27T15:51:07Z","updated_at":"2018-06-27T15:51:07Z","closed_at":null,"author_association":"MEMBER","body":"Extracted from comments on #26962:\r\n\r\nWe 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.\r\n\r\nFrom @andy-kimball:\r\n>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:\r\n>\r\n>    Multi-column constraints will always have exact prefixes factored out (according to ExactPrefix method):\r\n>\r\n>/y/m/d: [/2018/10/2 - /2018/11/2]\r\n>=>\r\n>/y: [/2018 - /2018]\r\n>/m/d: [/10/2 - /11/2]\r\n>\r\n>/y/m/d: [/2018/10/2 - /2018/10/5] [/2018/10/15 - /2018/10/20]\r\n>=>\r\n>/y: [/2018 - /2018]\r\n>/m: [/10 - /10]\r\n>/d: [/2 - /5] [/15 - /20]\r\n>\r\n>    Every column appearing in a multi-column constraint will also have a single-column constraint, assuming its single-col version is not unconstrained:\r\n>\r\n>/y/m/d: [/2018/10/2 - /2018/11/2]\r\n>=>\r\n>/y: [/2018 - /2018]\r\n>/m/d: [/10/2 - /11/2]\r\n>/m: [/10 - /11]\r\n>\r\n>/y/m/d: [/2018/10/2 - /2018/10/5] [/2020/10/12 - /2020/10/15]\r\n>=>\r\n>/y/m/d: [/2018/10/2 - /2018/10/5] [/2020/10/12 - /2020/10/15]\r\n>/y: [/2018 - /2018] [/2020 - /2020]\r\n>/m: [/10 - /10]\r\n>/d: [/2 - /5] [/12 - /15]\r\n>\r\n>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.\r\n\r\ncc @RaduBerinde @rytaft @madhavsuresh "}}
{"actor":{"display_login":"justinj"}}