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 <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Philip Zeyliger <[email protected]> Gerrit-HasComments: No
