Dan Hecht has posted comments on this change.

Change subject: IMPALA-5036: Parquet count star optimization
......................................................................


Patch Set 5:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/6812/5/fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java
File fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java:

Line 886:         prefix + 
"8InitZeroIN10impala_udf9BigIntValEEEvPNS2_15FunctionContextEPT_",
> I gave this a try, but ran into a problem. I tried substituting a count(*) 
You'd have to replace the uses of the agg slot with the zeroifnull() 
expression. If that's too complicated, then I'm fine with leaving it. I still 
think, conceptually, it's cleaner to be able to do rewrites using existing 
expressions rather than build new ones, but if it doesn't work out given our 
current optimizer, then so be it.


http://gerrit.cloudera.org:8080/#/c/6812/5/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java:

Line 109:  *
> This is the only one (I think it's pretty high level).
This comment doesn't explain the overall optimization. What I'm looking for is 
something that explains the optimization: what the rewrite is, and what the 
"special" scan is.


PS5, Line 140:  This
             :   // scan does additional analysis in init() to determine 
whether it is correct to apply
             :   // the optimization.
> An alternative approach that Alex and I are thinking about is passing in th
Okay. If it doesn't work out, I just think the comments needs to be clarified. 
Future readers won't be looking at this code review to understand the code.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I536b85c014821296aed68a0c68faadae96005e62
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky <tbobrovyt...@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com>
Gerrit-Reviewer: Lars Volker <l...@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <mar...@cloudera.com>
Gerrit-Reviewer: Mostafa Mokhtar <mmokh...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tbobrovyt...@cloudera.com>
Gerrit-Reviewer: Zach Amsden <zams...@cloudera.com>
Gerrit-HasComments: Yes

Reply via email to