Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/11760 )
Change subject: IMPALA-7655: Rewrite if, isnull, coalesce to use CASE ...................................................................... Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/11760/4/be/src/exprs/conditional-functions.h File be/src/exprs/conditional-functions.h: http://gerrit.cloudera.org:8080/#/c/11760/4/be/src/exprs/conditional-functions.h@a75 PS4, Line 75: High level comment (still digging into the code). All these rewrites are hidden behind a session option (ENABLE_EXPR_REWRITES). Removing these backend implementations essentially means that when someone sets ENABLE_EXPR_REWRITES=false, the conditional functions won't run (obvious since they aren't rewritten to the corresponding CASE form). If we fix that though, one thing I'm concerned here is that if something goes wrong with the rewrite rules (yes this happened in quite a few cases in the past), there doesn't seem to be any other workaround and all the queries with the problematic conditionals would fail. Wondering if we should still retain these be implementations until we are sure that this optimization works pretty well? -- To view, visit http://gerrit.cloudera.org:8080/11760 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I526654d8546e17b2545c42cc59dab66d9fe1b163 Gerrit-Change-Number: 11760 Gerrit-PatchSet: 4 Gerrit-Owner: Paul Rogers <par0...@yahoo.com> Gerrit-Reviewer: Bharath Vissapragada <bhara...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Paul Rogers <par0...@yahoo.com> Gerrit-Reviewer: Philip Zeyliger <phi...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Vuk Ercegovac <vercego...@cloudera.com> Gerrit-Comment-Date: Fri, 26 Oct 2018 18:23:12 +0000 Gerrit-HasComments: Yes
