Tim Armstrong has posted comments on this change. Change subject: IMPALA-1286: Extract common conjuncts from disjunctions. ......................................................................
Patch Set 5: Code-Review+1 I was less concerned with what the output type is and more about how it is obtained. The problem is that we don't have any comprehensible rules about how the output type of an expression is obtained or what the input to the type derivation algorithm even is. In my mind a query option shouldn't be part of the input to the type derivation. It's all a consequence of a pre-existing design so I'm not saying you should fix it as part of this change. I think the original sin is that the output type of an operation should be determined by the input types, because that gives you a reasonable way to trace back and understand how the type of an expression was derived. This isn't the case today since it looks at other information and maybe depends in complicate way on other rewrites. My point about dependent types is that you can achieve what you're arguing for without making typechecking depend on a query optimisation. E.g. the logical types would be "LITERAL(1) + LITERAL(1) => LITERAL(2)" instead of "TINYINT + TINYINT => SMALLINT) and they would be lowered at some point into actual physical types. The current system of typing literals seems to be trying to approximate this. I'm not saying we shouldn't go ahead, but I think there are design problems with the type system and I think this is digging the hole a little deeper. I'm ok as long as we have a way to replace it with a more principled approach later without breaking things. It seems like it's mostly corner cases that would be affected in the meantime and there probably is a way out. -- To view, visit http://gerrit.cloudera.org:8080/4877 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3cf9b950afaa3fd753d1b09ba5e540b5258940ad Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: No
