impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Philip Zeyliger (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5211: Simplifying ifnull conditional.
Date Wed, 06 Sep 2017 22:36:01 GMT
Philip Zeyliger has posted comments on this change.

Change subject: IMPALA-5211: Simplifying ifnull conditional.
......................................................................


Patch Set 5:

Alex,

Thanks for your reviews.

I put together a new diff which ties to do the re-write. The unit test passes, but I'm just
now kicking off the full tests.

That said, I've got two questions I could use your advice on:
* Am I right in thinking that I can now go and delete IFNULL code from the backend? 
* I'm worried about the types of NullLiteral. i.e., when I do the re-write I don't have the
types of expressions here. I'm worried there's nuance here I don't yet know about.

I also am beginning to suspect that I shouldn't be special-casing if. Instead, "x [not] distinct
from x -> false [true]" should be its own rule. Then this would apply in a few more cases.

After this conversation, we also think that https://impala.incubator.apache.org/docs/build/html/topics/impala_conditional_functions.html
is wrong about how it rewrites nullif. It uses "=" and not "<=>" and so is wrong about
nulls. I'm happy to update the docs or file a JIRA.

Thanks!

-- 
To view, visit http://gerrit.cloudera.org:8080/7829
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id91ca968a0c0be44e1ec54ad8602f91a5cb2e0e5
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Philip Zeyliger <philip@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <philip@cloudera.com>
Gerrit-HasComments: No

Mime
View raw message