Thomas Tauber-Marshall has posted comments on this change. Change subject: IMPALA-4731: Crash when sorting on non-deterministic expr ......................................................................
Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/5914/1//COMMIT_MSG Commit Message: Line 7: IMPALA-4731: Crash when sorting on non-deterministic expr > This also may fix IMPALA-4728 or IMPALA-397. It does fix IMPALA-397 It doesn't fix IMPALA-4728 as creation of the SortInfo for analytic functions doesn't go through this code path. Greg said that the main motivation behind IMPALA-4728, to_date() being slow, has been fixed now, so I think we can hold off on dealing with that one for now. http://gerrit.cloudera.org:8080/#/c/5914/1/fe/src/main/java/org/apache/impala/analysis/SortInfo.java File fe/src/main/java/org/apache/impala/analysis/SortInfo.java: Line 194: // entirely if this eliminates all the ordering exprs. > This is a weird corner case case, but I think we should drop LiteralExprs i Done Line 207: materializedOrderingExprs.add(materializedRef); > I think we only need to substitute the SlotRefs in the first branch of the Done http://gerrit.cloudera.org:8080/#/c/5914/1/tests/query_test/test_sort.py File tests/query_test/test_sort.py: Line 162: def test_order_by_random(self): > I think the example in IMPALA-397 would also be a good test - there rand() Done -- To view, visit http://gerrit.cloudera.org:8080/5914 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5dcda32fc7770d42fc500ce87fc54d58e5b5dc00 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Thomas Tauber-Marshall <tmarsh...@cloudera.com> Gerrit-Reviewer: Thomas Tauber-Marshall <tmarsh...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-HasComments: Yes
