impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dimitris Tsirogiannis (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-4166: Add SORT BY sql clause
Date Fri, 05 May 2017 05:14:05 GMT
Dimitris Tsirogiannis has posted comments on this change.

Change subject: IMPALA-4166: Add SORT BY sql clause

Patch Set 18:


Thanks Lars. Some minor comments on testing.
File testdata/workloads/functional-planner/queries/PlannerTest/ddl.test:

PS18, Line 183: # CTAS with partitions and sort by columns
If you don't have one already, can you add a test for a partitioned table with an empty sort
by clause (e.g. partitioned by (year) sort by ()). Also, you may want to check the following
1. create partitioned table with non-empty sort by clause
2. drop all the columns that appear in the sort by clause
3. check if this is equivalent to sort by ().
File testdata/workloads/functional-planner/queries/PlannerTest/insert-sort-by.test:

PS18, Line 327: # Test that using both the sortby hint and sort by columns work.
Thanks for adding this test. Can you also add another one in which theres is some overlap
between the columns of the sort by hint and the columns of the sort by clause?

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I08834f38a941786ab45a4381c2732d929a934f75
Gerrit-PatchSet: 18
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker <>
Gerrit-Reviewer: Alex Behm <>
Gerrit-Reviewer: Dimitris Tsirogiannis <>
Gerrit-Reviewer: Lars Volker <>
Gerrit-Reviewer: Marcel Kornacker <>
Gerrit-Reviewer: Thomas Tauber-Marshall <>
Gerrit-HasComments: Yes

View raw message