impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Vuk Ercegovac (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-4985: use parquet stats of nested types for dynamic pruning
Date Tue, 21 Nov 2017 00:33:26 GMT
Vuk Ercegovac has posted comments on this change. ( )

Change subject: IMPALA-4985: use parquet stats of nested types for dynamic pruning

Patch Set 4:

File fe/src/main/java/org/apache/impala/planner/
PS4, Line 179:   // A collection type is "required" if it is checked to be not empty. Collection
> Shrink? The following seems sufficient.
went with just referring to not-empty instead of not-empty and required.
PS4, Line 184:   // Analysis adds IsNotEmptyPredicates where needed to enforce required collections.
> I think this could be misunderstood. IsNotEmptyPredicates are not required,
PS4, Line 436:   // Checks if slot refers to an array "pos" pseudo-column.
> nit: we typically use /** */ style class and function comments
whoops, Done
PS4, Line 440:   private boolean isArrayPosReference(SlotRef slot) {
> Move to SlotRef. I generally prefer 'slotRef' instead of 'slot' because the
Done. sync'd with similar uses below as well.
PS4, Line 442:     if (parent != null) {
> reverse? if (parent == null) return false;
PS4, Line 533:    * Adds nested collections that are required from 'conjuncts' to requiredCollections_.
> Populates 'requiredCollections_' based on IsNotEmptyPredicates in the given
PS4, Line 537:    * Example: table T has field A which is of type array<array<int>>.
> I don't think examples really belong here because this is not where we are 
PS4, Line 548:         Preconditions.checkArgument(ref.getDesc().getType().isComplexType());
> checkState
PS4, Line 549:         Preconditions.checkArgument(ref.getDesc().getItemTupleDesc() != null);
> checkState
PS4, Line 574:       // Note: it is conservative (but correct) to depend on a collection to
be required.
> I think we should state what we mean more directly. We rely on analysis to 
PS4, Line 586:    * collection-typed slot. As conjuncts are seen, mark nested collections
that are
> mark -> collect
File testdata/workloads/functional-query/queries/QueryTest/nested-types-parquet-stats.test:
PS4, Line 33: aggregation(SUM, NumRowGroups): 2
> we should also have planner tests to see how the explain looks

To view, visit
To unsubscribe, visit

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0c99e20cb080b504442cd5376ea3e046016158fe
Gerrit-Change-Number: 8480
Gerrit-PatchSet: 4
Gerrit-Owner: Vuk Ercegovac <>
Gerrit-Reviewer: Alex Behm <>
Gerrit-Reviewer: Lars Volker <>
Gerrit-Reviewer: Vuk Ercegovac <>
Gerrit-Comment-Date: Tue, 21 Nov 2017 00:33:26 +0000
Gerrit-HasComments: Yes

  • Unnamed multipart/alternative (inline, 8-Bit, 0 bytes)
View raw message