Alex Behm has posted comments on this change.

Change subject: IMPALA-2328 Parquet scan should use min/max stats
......................................................................


Patch Set 1:

(3 comments)

Imo after following my suggested design this patch will require much less code.

http://gerrit.cloudera.org:8080/#/c/3623/1/be/src/exprs/anyval-util.cc
File be/src/exprs/anyval-util.cc:

Line 46:     case TYPE_MINMAX_TINYINT: return new MinMaxTinyIntVal;
We should definitely not introduce new types and/or Values for this purpose 
specifically. The problem can be more elegantly solved by generating 
appropriate predicates in the Frontend.


http://gerrit.cloudera.org:8080/#/c/3623/1/be/src/exprs/expr-value.h
File be/src/exprs/expr-value.h:

Line 42:   impala_udf::MinMaxVal<int8_t> minmax_tinyint_val;
We should definitely not introduce new types and/or Values for this purpose 
specifically. The problem can be more elegantly solved by generating 
appropriate predicates in the Frontend.


http://gerrit.cloudera.org:8080/#/c/3623/1/be/src/runtime/tuple-row.h
File be/src/runtime/tuple-row.h:

Line 75:   Tuple* tuples_[2];
I understand why you made this change for your patch, but this change should 
not be necessary if we do the min/max pruning based on a single tuple (with 
separate lists for the min and max predicates).


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I91de1f4d0fb2a982d06cd344e41901e3bf3c2cea
Gerrit-PatchSet: 1
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Jian Wu <[email protected]>
Gerrit-Reviewer: Alex Behm <[email protected]>
Gerrit-Reviewer: Jian Wu <[email protected]>
Gerrit-Reviewer: Michael Ho <[email protected]>
Gerrit-Reviewer: Mostafa Mokhtar <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-HasComments: Yes

Reply via email to