{"action":"created","issue":{"url":"https://api.github.com/repos/cockroachdb/cockroach/issues/3298","labels_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/3298/labels{/name}","comments_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/3298/comments","events_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/3298/events","html_url":"https://github.com/cockroachdb/cockroach/pull/3298","id":120052268,"number":3298,"title":"sql: stricter type checking on GREATEST and LEAST","user":{"login":"mjibson","id":41181,"avatar_url":"https://avatars.githubusercontent.com/u/41181?v=3","gravatar_id":"","url":"https://api.github.com/users/mjibson","html_url":"https://github.com/mjibson","followers_url":"https://api.github.com/users/mjibson/followers","following_url":"https://api.github.com/users/mjibson/following{/other_user}","gists_url":"https://api.github.com/users/mjibson/gists{/gist_id}","starred_url":"https://api.github.com/users/mjibson/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mjibson/subscriptions","organizations_url":"https://api.github.com/users/mjibson/orgs","repos_url":"https://api.github.com/users/mjibson/repos","events_url":"https://api.github.com/users/mjibson/events{/privacy}","received_events_url":"https://api.github.com/users/mjibson/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":3,"created_at":"2015-12-02T22:40:40Z","updated_at":"2015-12-03T06:05:33Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/cockroachdb/cockroach/pulls/3298","html_url":"https://github.com/cockroachdb/cockroach/pull/3298","diff_url":"https://github.com/cockroachdb/cockroach/pull/3298.diff","patch_url":"https://github.com/cockroachdb/cockroach/pull/3298.patch"},"body":"Change the returnType checking to return a func that can dynamically\r\ntype check based on the arguments it receives. This is required\r\nbecause GREATEST and LEAST can return either a float or an int, based\r\non their arguments. Before this change, lib/pq would panic under some\r\nconditions listed in #3294.\r\n\r\nTo prevent this, we need to make sure all the arguments to GREATEST\r\nand LEAST are of the same type. We could perform this check in the\r\nimplementation of GREATEST and LEAST themselves (which would trigger\r\nan error in the type checker), but this requires running the check\r\non all executions of those functions. Instead, we can perform it only\r\nonce during the type checking phase.\r\n\r\nfixes #3294\n\n<!-- Reviewable:start -->\n[<img src=\"https://reviewable.io/review_button.png\" height=40 alt=\"Review on Reviewable\"/>](https://reviewable.io/reviews/cockroachdb/cockroach/3298)\n<!-- Reviewable:end -->\n"},"comment":{"url":"https://api.github.com/repos/cockroachdb/cockroach/issues/comments/161526365","html_url":"https://github.com/cockroachdb/cockroach/pull/3298#issuecomment-161526365","issue_url":"https://api.github.com/repos/cockroachdb/cockroach/issues/3298","id":161526365,"user":{"login":"mjibson","id":41181,"avatar_url":"https://avatars.githubusercontent.com/u/41181?v=3","gravatar_id":"","url":"https://api.github.com/users/mjibson","html_url":"https://github.com/mjibson","followers_url":"https://api.github.com/users/mjibson/followers","following_url":"https://api.github.com/users/mjibson/following{/other_user}","gists_url":"https://api.github.com/users/mjibson/gists{/gist_id}","starred_url":"https://api.github.com/users/mjibson/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mjibson/subscriptions","organizations_url":"https://api.github.com/users/mjibson/orgs","repos_url":"https://api.github.com/users/mjibson/repos","events_url":"https://api.github.com/users/mjibson/events{/privacy}","received_events_url":"https://api.github.com/users/mjibson/received_events","type":"User","site_admin":false},"created_at":"2015-12-03T06:05:33Z","updated_at":"2015-12-03T06:05:33Z","body":"\n\n\n\nReview status: 1 of 3 files reviewed at latest revision, 5 unresolved discussions.\n\n---\n\n*[sql/parser/builtins.go, line 1257 \\[r1\\]](https://reviewable.io:443/reviews/cockroachdb/cockroach/3298#-K4Zs-SfJWL1K4tRMXmH:-K4aOSlvgGbtn-E22fMp:-499146009) ([raw file](https://github.com/cockroachdb/cockroach/blob/cb9449e98394e76c0142b869d50d514025cd43e4/sql/parser/builtins.go#L1257)):*\nDone.\n\n---\n\n*[sql/parser/builtins.go, line 1258 \\[r1\\]](https://reviewable.io:443/reviews/cockroachdb/cockroach/3298#-K4ZsZMGOZ_eLuKWcGL9:-K4aQ9fzh5appihtgfUe:-499146009) ([raw file](https://github.com/cockroachdb/cockroach/blob/cb9449e98394e76c0142b869d50d514025cd43e4/sql/parser/builtins.go#L1258)):*\nDone.\n\n---\n\n*[sql/parser/builtins.go, line 1265 \\[r1\\]](https://reviewable.io:443/reviews/cockroachdb/cockroach/3298#-K4ZqhqRYysnN0_En8ve-r1-1265:-K4aQA5QjmQr5RXjtEEA:-499146009) ([raw file](https://github.com/cockroachdb/cockroach/blob/cb9449e98394e76c0142b869d50d514025cd43e4/sql/parser/builtins.go#L1265)):*\nDone.\n\n---\n\n*[sql/parser/builtins.go, line 1270 \\[r1\\]](https://reviewable.io:443/reviews/cockroachdb/cockroach/3298#-K4ZqhqRYysnN0_En8ve-r1-1270:-K4aQBMgV6YcujEymHns:507933655) ([raw file](https://github.com/cockroachdb/cockroach/blob/cb9449e98394e76c0142b869d50d514025cd43e4/sql/parser/builtins.go#L1270)):*\nI refactored the loop to avoid this problem. And since datum starts as DNull, that's what will be returned if all arguments are null.\n\n---\n\n*[sql/parser/type_check.go, line 29 \\[r1\\]](https://reviewable.io:443/reviews/cockroachdb/cockroach/3298#-K4ZqhqS15rUFyd8Il_f-r1-29:-K4aRB9CvtiuAIUGPq1Z:-499146009) ([raw file](https://github.com/cockroachdb/cockroach/blob/cb9449e98394e76c0142b869d50d514025cd43e4/sql/parser/type_check.go#L29)):*\nDone.\n\n---\n\n\n*Comments from the [review on Reviewable.io](https://reviewable.io:443/reviews/cockroachdb/cockroach/3298)*\n<!-- Sent from Reviewable.io -->\n"}}