Philip Zeyliger has posted comments on this change. Change subject: IMPALA-5211: Simplifying ifnull/isnull/nvl where conditional is a literal. ......................................................................
Patch Set 3: > Matt: Might be nice to have some tests like those added in this patch which > make sure the rewritten queries work with nullable tuples (which are a weird > wart, arguably): The bug there was, if I read it right, that coalesce(non-nullable-column, x) was getting simplified to x, but the column could have been coming from an outer join. Since this change deals only in literals (and not columnns), I decided that adding a query test wasn't that valuable. If you disagree, I'd be happy to add one. I also wanted to see if I could tell from the explain output whether or not the optimization was happening. It looks like I can, if I put the relevant expression inside of an aggregation, since "output: max:merge(id)" shows up. This could also power a test. Do you think it's worth it? [...:21000] > explain select max(nvl(null, id)) from functional.alltypes limit 3; Query: explain select max(nvl(null, id)) from functional.alltypes limit 3 +----------------------------------------------+ | Explain String | +----------------------------------------------+ | Max Per-Host Resource Reservation: Memory=0B | | Per-Host Resource Estimates: Memory=180.00MB | | Codegen disabled by planner | | | | PLAN-ROOT SINK | | | | | 03:AGGREGATE [FINALIZE] | | | output: max:merge(id) | | | limit: 3 | | | | | 02:EXCHANGE [UNPARTITIONED] | | | | | 01:AGGREGATE | | | output: max(id) | | | | | 00:SCAN HDFS [functional.alltypes] | | partitions=24/24 files=24 size=478.45KB | +----------------------------------------------+ Fetched 17 row(s) in 0.02s -- To view, visit http://gerrit.cloudera.org:8080/7781 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9e4b8bf6fedd595f9ea54ffb30fc71a058c7f16c Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Philip Zeyliger <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Henry Robinson <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Philip Zeyliger <[email protected]> Gerrit-HasComments: No
