Mailing-List: contact reviews-help@impala.incubator.apache.org; run by ezmlm
Precedence: bulk
Message-Id: <201709062236.v86Ma1vX005629@ip-10-146-233-104.ec2.internal>
Date: Wed, 6 Sep 2017 22:36:01 +0000
From: "Philip Zeyliger (Code Review)" <gerrit@cloudera.org>
To: impala-cr@cloudera.com, reviews@impala.incubator.apache.org
CC: Alex Behm <alex.behm@cloudera.com>, Matthew Jacobs <mj@cloudera.com>
Reply-To: philip@cloudera.com
Subject: =?UTF-8?Q?=5BImpala-ASF-CR=5D_IMPALA-5211=3A_Simplifying_ifnull_conditional=2E=0A?=
In-Reply-To: <gerrit.1503686704000.Id91ca968a0c0be44e1ec54ad8602f91a5cb2e0e5@gerrit.cloudera.org>
References: <gerrit.1503686704000.Id91ca968a0c0be44e1ec54ad8602f91a5cb2e0e5@gerrit.cloudera.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Content-Disposition: inline
User-Agent: Gerrit/2.12.7
archived-at: Wed, 06 Sep 2017 22:36:06 -0000

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