Tim Armstrong has posted comments on this change. Change subject: IMPALA-1286: Extract common conjuncts from disjunctions. ......................................................................
Patch Set 5: I understand that it's harmless in that queries that succeeded before the patch won't start failing after the patch, but I'm concerned that a) queries that succeed with optimisation on will fail or behave differently when you turn optimisations off because the analyser picks a wider type and b) the lack of distinction between user-visible types and "internal" types will be a source of bugs and confusing behaviour. There may only be a couple of cases where this is user-visible (CTAS and typeof() come to mind), so maybe there's a way to fix that there. The longer-term design concern is if there aren't clear rules for determining the types of expressions that can be applied during the initial semantic analysis (e.g. BOOL || BOOL => BOOL), it makes it very hard to change the implementation of the frontend without changing the user-visible behaviour of the frontend. -- 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
