Lars Volker has posted comments on this change.

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


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7073/2/testdata/workloads/functional-planner/queries/PlannerTest/order.test
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 SortInfo.java 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.


http://gerrit.cloudera.org:8080/#/c/7073/2/testdata/workloads/functional-planner/queries/PlannerTest/tpch-nested.test
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 (exec-node.cc:449). 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 http://gerrit.cloudera.org:8080/7073
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

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

Reply via email to