{"action":"created","issue":{"url":"https://api.github.com/repos/cockroachdb/cockroach/issues/24704","repository_url":"https://api.github.com/repos/cockroachdb/cockroach","labels_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/24704/labels{/name}","comments_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/24704/comments","events_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/24704/events","html_url":"https://github.com/cockroachdb/cockroach/pull/24704","id":313461261,"number":24704,"title":"distsql: Remove column size check from INTERSECT and EXCEPT joins","user":{"login":"abhimadan","id":7970201,"avatar_url":"https://avatars2.githubusercontent.com/u/7970201?v=4","gravatar_id":"","url":"https://api.github.com/users/abhimadan","html_url":"https://github.com/abhimadan","followers_url":"https://api.github.com/users/abhimadan/followers","following_url":"https://api.github.com/users/abhimadan/following{/other_user}","gists_url":"https://api.github.com/users/abhimadan/gists{/gist_id}","starred_url":"https://api.github.com/users/abhimadan/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/abhimadan/subscriptions","organizations_url":"https://api.github.com/users/abhimadan/orgs","repos_url":"https://api.github.com/users/abhimadan/repos","events_url":"https://api.github.com/users/abhimadan/events{/privacy}","received_events_url":"https://api.github.com/users/abhimadan/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":2,"created_at":"2018-04-11T19:43:37Z","updated_at":"2018-04-11T21:09:40Z","closed_at":null,"author_association":"MEMBER","pull_request":{"url":"https://api.github.com/repos/cockroachdb/cockroach/pulls/24704","html_url":"https://github.com/cockroachdb/cockroach/pull/24704","diff_url":"https://github.com/cockroachdb/cockroach/pull/24704.diff","patch_url":"https://github.com/cockroachdb/cockroach/pull/24704.patch"},"body":"This check mistakenly used the joiner's `PostProcessSpec` to verify that\r\nall left and right columns are used as equality columns. If there's a\r\nprojection or render on the joiner, this check doesn't work as intended,\r\nsince the output columns don't match the input columns. It's not\r\npossible for a processor constructor to differentiate its input's merge\r\nordering columns from its \"actual\" output columns when a processor is\r\nconstructed, and the physical planner already guarantees that all\r\nequality columns are output columns. Therefore, the check isn't\r\nnecessary, and has been removed.\r\n\r\nAlso add subtests to the `distsql_union` logic test file to split up the\r\ndifferent set operations.\r\n\r\nFixes #24689.\r\n\r\nRelease note: None"},"comment":{"url":"https://api.github.com/repos/cockroachdb/cockroach/issues/comments/380596607","html_url":"https://github.com/cockroachdb/cockroach/pull/24704#issuecomment-380596607","issue_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/24704","id":380596607,"user":{"login":"solongordon","id":679246,"avatar_url":"https://avatars1.githubusercontent.com/u/679246?v=4","gravatar_id":"","url":"https://api.github.com/users/solongordon","html_url":"https://github.com/solongordon","followers_url":"https://api.github.com/users/solongordon/followers","following_url":"https://api.github.com/users/solongordon/following{/other_user}","gists_url":"https://api.github.com/users/solongordon/gists{/gist_id}","starred_url":"https://api.github.com/users/solongordon/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/solongordon/subscriptions","organizations_url":"https://api.github.com/users/solongordon/orgs","repos_url":"https://api.github.com/users/solongordon/repos","events_url":"https://api.github.com/users/solongordon/events{/privacy}","received_events_url":"https://api.github.com/users/solongordon/received_events","type":"User","site_admin":false},"created_at":"2018-04-11T21:09:40Z","updated_at":"2018-04-11T21:09:40Z","author_association":"CONTRIBUTOR","body":"Nice, happy this was a straightforward fix. Some testing complaints but otherwise looks good.\n\n---\n\nReview status: 0 of 2 files reviewed at latest revision, all discussions resolved.\n\n---\n\n*[pkg/sql/logictest/testdata/logic_test/distsql_union, line 397 at r1](https://beta.reviewable.io/reviews/cockroachdb/cockroach/24704#-L9qWYGd5I-xrN6VC20Y:-L9qWYGd5I-xrN6VC20Z:bbyxkf7) ([raw file](https://github.com/cockroachdb/cockroach/blob/6b230834fb2de5a0d956dc9c70835e384d960b5c/pkg/sql/logictest/testdata/logic_test/distsql_union#L397)):*\n> ```\n> 2\n> \n> # INTERSECT ALL and INTERSECT with a projection on the result.\n> ```\n\nIt doesn't seem like your new tests are exercising your fix. I tried running them against master and they still passed. Could you add some that failed before and pass now?\n\n---\n\n*[pkg/sql/logictest/testdata/logic_test/distsql_union, line 402 at r1](https://beta.reviewable.io/reviews/cockroachdb/cockroach/24704#-L9qXLuiE-g-Q2zz5IRO:-L9qXLuiE-g-Q2zz5IRP:b52x549) ([raw file](https://github.com/cockroachdb/cockroach/blob/6b230834fb2de5a0d956dc9c70835e384d960b5c/pkg/sql/logictest/testdata/logic_test/distsql_union#L402)):*\n> ```\n> ----\n> 1\n> 5\n> ```\n\nNit: Apparently this doesn't matter with `rowsort` enabled, but why not sort the expected rows?\n\n---\n\n\n*Comments from [Reviewable](https://beta.reviewable.io/reviews/cockroachdb/cockroach/24704#-:-L9qX_Ql4r9bREcvGmt5:b-cr2svm)*\n<!-- Sent from Reviewable.io -->\n"}}
{"actor":{"display_login":"solongordon"}}