Vuk Ercegovac 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: (3 comments) main question from my end is to consider what happens when rewrites are disabled. http://gerrit.cloudera.org:8080/#/c/11760/4/common/function-registry/impala_functions.py File common/function-registry/impala_functions.py: http://gerrit.cloudera.org:8080/#/c/11760/4/common/function-registry/impala_functions.py@531 PS4, Line 531: # Rewritten away in the FE can you explain why this is needed in this change? do we assert anywhere that these fns are never seen by the backend? http://gerrit.cloudera.org:8080/#/c/11760/4/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java File fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java: http://gerrit.cloudera.org:8080/#/c/11760/4/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java@a92 PS4, Line 92: rewrites can be disabled via a query option so cannot be assumed to run. it cannot be assumed to transform an invalid/unknown stmt to a valid one. its purpose is only to transform valid statements to other equivalent valid statements. what happens when you do: set enable_expr_rewrites = false ? http://gerrit.cloudera.org:8080/#/c/11760/4/fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java File fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java: http://gerrit.cloudera.org:8080/#/c/11760/4/fe/src/main/java/org/apache/impala/rewrite/SimplifyConditionalsRule.java@112 PS4, Line 112: <code> to be consistent with the style in the frontend Java code, pls remove these javadoc markups. I think the idea is that the code is read through a variety of editors and since the javadocs are not used, simplify writing/reading the comments by omitting the markup. -- 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: 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: Wed, 24 Oct 2018 21:13:20 +0000 Gerrit-HasComments: Yes
