impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Lars Volker (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-3643/IMPALA-5344: Fix FE tests on Java 8
Date Mon, 05 Jun 2017 17:29:52 GMT
Lars Volker has posted comments on this change.

Change subject: IMPALA-3643/IMPALA-5344: Fix FE tests on Java 8

Patch Set 2:

File testdata/workloads/functional-planner/queries/PlannerTest/order.test:

Line 694: |  pass-through-operands: all
> I'm confused why this changed at all. The number of pass through operators 
I debugged this to find out why this changes. The child nodes need to pass a filter whether
they can be passed through or not. One of the criteria is that all slot descriptors need to
be the same, and before my change, some slots in the sort node had different offsets. I checked and found a loop over the source slots using a HashSet (Line 193):

    Set<SlotRef> sourceSlots = Sets.newHashSet();

This means the sort node would reorder slots, thus preventing the pass through. After changing
this to a LinkedHashSet, the insertion order would be preserved, thus the slot offsets wouldn't
change, and the check for pass-through-ability passed.
File testdata/workloads/functional-planner/queries/PlannerTest/tpch-nested.test:

Line 2411:    predicates on o: !empty(o.o_lineitems), o_orderstatus = 'F'
> I think I don't quite understand - even if conjuncts are evaluated on mater
You're right, evaluation is aborted early if one fails ( The order here
is the order in which conjuncts are added to the analyzer and changes because GlobalState.conjuncts
turned into a LinkedHashMap. Now it is somewhat more deterministic than before, but I agree
that this can change performance for the better or worse.

As discussed offline I'll kick of a private perf run. Thank you for the suggestion.

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad8e1dccec3a51293a109c420bd2b88b9d1e0625
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker <>
Gerrit-Reviewer: Jim Apple <>
Gerrit-Reviewer: Lars Volker <>
Gerrit-Reviewer: Tim Armstrong <>
Gerrit-HasComments: Yes

View raw message