impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Philip Zeyliger (Code Review)" <>
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:


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
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.


To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: Id91ca968a0c0be44e1ec54ad8602f91a5cb2e0e5
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Philip Zeyliger <>
Gerrit-Reviewer: Alex Behm <>
Gerrit-Reviewer: Matthew Jacobs <>
Gerrit-Reviewer: Philip Zeyliger <>
Gerrit-HasComments: No

View raw message