Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/13955 )
Change subject: IMPALA-8755: Frontend support for Z-ordering ...................................................................... Patch Set 6: (4 comments) http://gerrit.cloudera.org:8080/#/c/13955/6/common/thrift/DataSinks.thrift File common/thrift/DataSinks.thrift: http://gerrit.cloudera.org:8080/#/c/13955/6/common/thrift/DataSinks.thrift@83 PS6, Line 83: // Sorting algorithm. If not lexical, the backend should not populate the : // RowGroup::sorting_columns list in parquet files. : 7: required Types.TSortingAlgorithm sorting_algorithm I think it's rather an 'order' than an 'algorithm'. Algorithm is e.g. quicksort that sorts your data into some specified order. Since you use it quite excessively I think the easiest way to search and replace it is to download the patch from gerrit, edit the patch file and replace 'algorithm' to 'order' and then apply the patch to a new branch. nit: I think you can leave out stuff about Parquet because it's just some low-level detail. http://gerrit.cloudera.org:8080/#/c/13955/6/fe/src/main/cup/sql-parser.cup File fe/src/main/cup/sql-parser.cup: http://gerrit.cloudera.org:8080/#/c/13955/6/fe/src/main/cup/sql-parser.cup@1399 PS6, Line 1399: RESULT = new CreateTableLikeStmt(tbl_def.getTblName(), nit: whitespaces at line end http://gerrit.cloudera.org:8080/#/c/13955/6/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java File fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java: http://gerrit.cloudera.org:8080/#/c/13955/6/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java@2570 PS6, Line 2570: nit: missing words http://gerrit.cloudera.org:8080/#/c/13955/6/testdata/workloads/functional-query/queries/QueryTest/create-table-like-table-zorder.test File testdata/workloads/functional-query/queries/QueryTest/create-table-like-table-zorder.test: http://gerrit.cloudera.org:8080/#/c/13955/6/testdata/workloads/functional-query/queries/QueryTest/create-table-like-table-zorder.test@24 PS6, Line 24: string_col We shouldn't allow string cols to be sorted by ZORDER. It's better to prohibit it during analysis. -- To view, visit http://gerrit.cloudera.org:8080/13955 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie122002ca8f52ca2c1e1ec8ff1d476ae1f4f875d Gerrit-Change-Number: 13955 Gerrit-PatchSet: 6 Gerrit-Owner: Norbert Luksa <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Norbert Luksa <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Fri, 09 Aug 2019 16:46:45 +0000 Gerrit-HasComments: Yes
