Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11556 )
Change subject: IMPALA-6323 Allow constant analytic window expressions. ...................................................................... Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/11556/5/be/src/exec/analytic-eval-node.cc File be/src/exec/analytic-eval-node.cc: http://gerrit.cloudera.org:8080/#/c/11556/5/be/src/exec/analytic-eval-node.cc@397 PS5, Line 397: && !(order_by_eq_expr_eval_ == nullptr || > I can look into that, but at first glance it seems sneaky. I'd prefer to r I'd think of it as normalising the ways we can represent equivalent operations rather than an unprincipled hack. Would it feel less sneaky if we documented (and maybe asserted on) the invariant that order_by_eq_expr_eval_ is non-NULL if window_end is set? This invariant was previously true and depended on by this code. -- To view, visit http://gerrit.cloudera.org:8080/11556 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Idf129026c45120e9470df601268863634037908c Gerrit-Change-Number: 11556 Gerrit-PatchSet: 5 Gerrit-Owner: Michal Ostrowski <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Michal Ostrowski <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Vuk Ercegovac <[email protected]> Gerrit-Comment-Date: Tue, 23 Oct 2018 16:02:45 +0000 Gerrit-HasComments: Yes
