Alex Behm has posted comments on this change. Change subject: IMPALA-5188: Add slot sorting in TupleDescriptor::LayoutEquals() ......................................................................
Patch Set 2: Code-Review+1 (3 comments) http://gerrit.cloudera.org:8080/#/c/6610/2/be/src/runtime/descriptors.cc File be/src/runtime/descriptors.cc: Line 347: vector<SlotDescriptor*> slots = this->slots(); can remove this-> Line 353: // slots on 'slot_idx'. We define a struct that wraps the comparison function. Can remove the last sentence (obvious from code) http://gerrit.cloudera.org:8080/#/c/6610/2/testdata/workloads/functional-query/queries/QueryTest/union.test File testdata/workloads/functional-query/queries/QueryTest/union.test: Line 1125: # IMPALA-5188: The slot descriptors might be ordered differently for the operands and the > Several queries were already added as part of the union passthrough patch ( This is a very specific bug. Imo, we should check in the min repro of a bug, and not necessarily the original query that found it. Of course, we need to be sufficiently confident that the simplified query covers the same issue, in particular, we should manually test that the original query is fixed by this patch. -- To view, visit http://gerrit.cloudera.org:8080/6610 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I426ad244678dbfe517262dfb7bbf4adc0247a35e Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Taras Bobrovytsky <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Michael Brown <[email protected]> Gerrit-Reviewer: Taras Bobrovytsky <[email protected]> Gerrit-HasComments: Yes
