Taras Bobrovytsky has posted comments on this change.

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


Patch Set 4:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/7073/4/testdata/workloads/functional-planner/queries/PlannerTest/lineage.test
File testdata/workloads/functional-planner/queries/PlannerTest/lineage.test:

Line 35:             "vertexId":"functional.alltypes.tinyint_col"
Do we know why the order changes in this file?


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 debugged this to find out why this changes. The child nodes need to pass 
I don't think there's a bug, Tim. Before this patch, passthrough was not 
enabled for some cases where it could be, (because the tuples are not identical 
even though they could be). It looks like this patch fixes the problem, this is 
great! Passthrough will be enabled in more cases now, making Impala faster.

Also, I don't think that we lose that much coverage. We have tests in for both 
passthrough and non-passthrough cases in union.test. In general, we expect the 
union node to be in passthrough mode most of the time.


Line 694: |  pass-through-operands: all
> Oh, cool, the reordering of the slots in the sort node seems unintentional.
Yes, it used to be unintentional, which looks like it's going to be fixed now.


-- 
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: 4
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: Taras Bobrovytsky <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-HasComments: Yes

Reply via email to