impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Lars Volker (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5144: Remove sortby() hint
Date Tue, 16 May 2017 13:44:53 GMT
Lars Volker has posted comments on this change.

Change subject: IMPALA-5144: Remove sortby() hint
......................................................................


Patch Set 2:

(3 comments)

Thank you for having a look. I added a test in PS3, and inline comments pointing to the remaining
tests you asked for.

http://gerrit.cloudera.org:8080/#/c/6885/2/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java:

PS2, Line 1792: 
> Do we have any tests that use shuffle and clustered? If not, maybe we shoul
Tests for shuffle and clustered are above this removed block, starting in Line 1726. The tests
here have been added for the sortby() hint in particular, and were also only executed for
non-legacy syntax. The tests above run for all hint syntaxes.


http://gerrit.cloudera.org:8080/#/c/6885/2/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
File fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java:

PS2, Line 508: 
> Since you're at it, maybe add a simply test with clustered as well. Didn't 
Good idea, done.


http://gerrit.cloudera.org:8080/#/c/6885/2/testdata/workloads/functional-planner/queries/PlannerTest/insert-sort-by.test
File testdata/workloads/functional-planner/queries/PlannerTest/insert-sort-by.test:

PS2, Line 328: 
> Same comment as before. Just make sure we have some coverage for shuffle + 
They are covered in multiple combinations in insert.test.


-- 
To view, visit http://gerrit.cloudera.org:8080/6885
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I83e1cd6fa7039035973676322deefbce00d3f594
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker <lv@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message