Marcel Kornacker has posted comments on this change. Change subject: IMPALA-4166: Add SORT BY sql clause ......................................................................
Patch Set 25: Code-Review+2 (4 comments) http://gerrit.cloudera.org:8080/#/c/6495/22/fe/src/main/java/org/apache/impala/analysis/TableDef.java File fe/src/main/java/org/apache/impala/analysis/TableDef.java: Line 295: when you're output error messages, it's probably better to refer to the exact syntactic element. so "SORT BY column list" is clearer than "sort column list". please apply elsewhere, in case i missed something. Line 352: "Please retry without caching: CREATE TABLE ... UNCACHED", why not "SORT BY is not supported..."? http://gerrit.cloudera.org:8080/#/c/6495/25/fe/src/main/java/org/apache/impala/analysis/TableDef.java File fe/src/main/java/org/apache/impala/analysis/TableDef.java: Line 291: TableDef.analyzeSortColumns(sortCols, do you need the 'tabledef.' prefix here? http://gerrit.cloudera.org:8080/#/c/6495/22/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java File fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java: Line 75: String sortByKey = AlterTableSortByStmt.TBL_PROP_SORT_COLUMNS; > I've discussed that with Alex and Dimitris and we no better place has emerg aren't you implying that it should live in hdfstable as well? -- To view, visit http://gerrit.cloudera.org:8080/6495 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I08834f38a941786ab45a4381c2732d929a934f75 Gerrit-PatchSet: 25 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Lars Volker <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]> Gerrit-HasComments: Yes
