Tim Armstrong has posted comments on this change. Change subject: IMPALA-5648: fix count(*) mem estimate regression ......................................................................
Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/7783/3/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java: Line 131: // overhead of MemPools, RowBatches, etc. This is only used if there is no data being > You say this is only used if there is no data being scanned, but I don't se Fair point. I reworded the comment. I think just setting a floor on the memory consumption is simplest. It mostly won't matter that much, it's just that outputting 0 looks "wrong". Line 136: private static final long MIN_MEMORY_ESTIMATE = 1 * 1024 * 1024; > final static (for consistency) I'd never noticed that we used the other way. "final static" sounds wrong to me. Looks both crop up. I'll make this file consistent at least. tarmstrong@tarmstrong-box:~/Impala/incubator-impala$ git grep 'static final' | wc -l 232 tarmstrong@tarmstrong-box:~/Impala/incubator-impala$ git grep 'final static' | wc -l 176 Line 1046: if ((slot.getColumn() == null || > How about this instead: Done -- To view, visit http://gerrit.cloudera.org:8080/7783 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iaf5c2316bef2afae54a94245c715534ed294f286 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
