You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Norbert Luksa (Code Review)" <ge...@cloudera.org> on 2019/08/06 08:25:21 UTC
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Norbert Luksa has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13955
Change subject: IMPALA-8755: Frontend support for Z-ordering
......................................................................
IMPALA-8755: Frontend support for Z-ordering
Extended the SQL grammar with an optional flag for SORT BY, ZORDER.
If set, the new 'sort.algorithm' table property will be set to ZORDER
and the information will sink down to the backend. The default
algorithm is indicated by LEXICAL and can be omitted. Examples are:
CREATE TABLE t (a INT, b INT) PARTITIONED BY (c INT)
SORT BY ZORDER (a, b);
CREATE TABLE t SORT BY ZORDER (int_col,id) LIKE u;
CREATE TABLE t LIKE PARQUET '/foo' SORT BY ZORDER (id,zip);
ALTER TABLE t SORT BY ZORDER (int_col,id);
The following two are the same queries:
CREATE TABLE t (a INT, b INT) SORT BY (a, b);
CREATE TABLE t (a INT, b INT) SORT BY LEXICAL (a, b);
Currently ZORDER has the same functionality as a simple SORT BY clause,
therefore hidden behind a feature flag: unlock_zorder. The custom
sorting with Z-ordering will be in a different commit later.
Testing:
* Added tests for the ZORDER option for every SORT BY test.
* Modified some tests by adding the LEXICAL option.
* The .test workloads are temporarily put in separate test files
in order to set up the feature flag. These tests are run from
tests/custom_cluster/test_zorder.py which is a duplication of
the relevant tests, but with CustomClusterTestSuite decorator.
Change-Id: Ie122002ca8f52ca2c1e1ec8ff1d476ae1f4f875d
---
M be/src/common/global-flags.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M common/thrift/DataSinks.thrift
M common/thrift/JniCatalog.thrift
M common/thrift/PlanNodes.thrift
M common/thrift/Types.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSortByStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/SortInfo.java
M fe/src/main/java/org/apache/impala/analysis/TableDef.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java
M fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java
M fe/src/main/java/org/apache/impala/planner/ExchangeNode.java
M fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/planner/SortNode.java
M fe/src/main/java/org/apache/impala/planner/TableSink.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/jflex/sql-scanner.flex
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlUtilsTest.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/insert-sort-by-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/alter-table-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-as-select-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-like-file-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-like-table-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/show-create-table-zorder.test
M testdata/workloads/functional-query/queries/QueryTest/show-create-table.test
A tests/custom_cluster/test_zorder.py
44 files changed, 1,749 insertions(+), 146 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/55/13955/5
--
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: newchange
Gerrit-Change-Id: Ie122002ca8f52ca2c1e1ec8ff1d476ae1f4f875d
Gerrit-Change-Number: 13955
Gerrit-PatchSet: 5
Gerrit-Owner: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/13955 )
Change subject: IMPALA-8755: Frontend support for Z-ordering
......................................................................
Patch Set 11:
(10 comments)
http://gerrit.cloudera.org:8080/#/c/13955/9//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13955/9//COMMIT_MSG@30
PS9, Line 30:
> For me, 'unlock' sounds more like the word I'd use with an experimental fea
hmm, indeed, unlock_zorder_sort would be slightly better. (nit)
http://gerrit.cloudera.org:8080/#/c/13955/9/fe/src/main/cup/sql-parser.cup
File fe/src/main/cup/sql-parser.cup:
http://gerrit.cloudera.org:8080/#/c/13955/9/fe/src/main/cup/sql-parser.cup@1399
PS9, Line 1399: CreateTableLikeStmt
> The null passed previously was the the sorting columns, therefore in this c
Thx for the explanation!
http://gerrit.cloudera.org:8080/#/c/13955/9/fe/src/main/cup/sql-parser.cup@1479
PS9, Line 1479: TSortingOrder.LEXICAL
> NOT_APPLICABLE is also an option, since any new sorting setting would requi
Ok, I'm fine with that.
http://gerrit.cloudera.org:8080/#/c/13955/9/fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
File fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java:
http://gerrit.cloudera.org:8080/#/c/13955/9/fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java@213
PS9, Line 213: "supported for Kudu tables.");
: }
:
> As far as I understand, this function is called when the 'sort.columns' or
I see. It's fine as it is.
http://gerrit.cloudera.org:8080/#/c/13955/9/fe/src/main/java/org/apache/impala/analysis/TableDef.java
File fe/src/main/java/org/apache/impala/analysis/TableDef.java:
http://gerrit.cloudera.org:8080/#/c/13955/9/fe/src/main/java/org/apache/impala/analysis/TableDef.java@397
PS9, Line 397: throw new AnalysisException(String.format("SORT BY ZORDER does not support "
: + "column type: %s", colType.toSql()));
> Yes, it raises an exception when finds the first one. Would it be better to
Hmm, It might be more usable if you printed out all the unsupported columns here as users would have to run multiple loops of query rewrite - re-execution if the run this on multiple unsupported columns.
As I see this is not a listing of particular columns just the unsupported types so it won't overwhelm the output if we printed all of them here. What do you think?
http://gerrit.cloudera.org:8080/#/c/13955/9/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
File fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java:
http://gerrit.cloudera.org:8080/#/c/13955/9/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@315
PS9, Line 315: TSortingOrder sortingOrder = TSortingOrder.valueOf(getSortingOrder(properties));
> Most of the variables in this function are only needed in the return clause
Sure
http://gerrit.cloudera.org:8080/#/c/13955/11/fe/src/main/java/org/apache/impala/planner/PlanNode.java
File fe/src/main/java/org/apache/impala/planner/PlanNode.java:
http://gerrit.cloudera.org:8080/#/c/13955/11/fe/src/main/java/org/apache/impala/planner/PlanNode.java@641
PS11, Line 641: getOrderExplainString
nit: What about getSortingOrderExplainString() ?
http://gerrit.cloudera.org:8080/#/c/13955/11/fe/src/test/java/org/apache/impala/analysis/ParserTest.java
File fe/src/test/java/org/apache/impala/analysis/ParserTest.java:
http://gerrit.cloudera.org:8080/#/c/13955/11/fe/src/test/java/org/apache/impala/analysis/ParserTest.java@2635
PS11, Line 2635: Baz
nit: Baz? Isn't this meant to be "Bar"?
http://gerrit.cloudera.org:8080/#/c/13955/11/tests/custom_cluster/test_zorder.py
File tests/custom_cluster/test_zorder.py:
http://gerrit.cloudera.org:8080/#/c/13955/11/tests/custom_cluster/test_zorder.py@29
PS11, Line 29: Temporary
Could you explain why this is temporary? If this goes away when the rest of the feature arrives could you add a TODO here?
http://gerrit.cloudera.org:8080/#/c/13955/11/tests/custom_cluster/test_zorder.py@122
PS11, Line 122: Runs a show-create-table test file, containing the following sections:
:
: ---- CREATE_TABLE
: contains a table creation statement to create table TABLE_NAME
: ---- RESULTS
: contains the expected result of SHOW CREATE TABLE table_name
:
: OR
:
: ---- CREATE_VIEW
: contains a view creation statement to create table VIEW_NAME
: ---- RESULTS
: contains the expected result of SHOW CREATE VIEW table_name
:
: OR
:
: ---- QUERY
: a show create table query
: ---- RESULTS
: contains the expected output of the SHOW CREATE TABLE query
nit: Could you re-phrase this function comment? I feel this is a bit overwhelming and could be expressed with some easy to read sentences.
--
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: 11
Gerrit-Owner: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 27 Aug 2019 09:58:52 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Posted by "Norbert Luksa (Code Review)" <ge...@cloudera.org>.
Norbert Luksa has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/13955 )
Change subject: IMPALA-8755: Frontend support for Z-ordering
......................................................................
IMPALA-8755: Frontend support for Z-ordering
Extended the SQL grammar with an optional flag for SORT BY, ZORDER.
If set, the new 'sort.algorithm' table property will be set to ZORDER
and the information will sink down to the backend. The default
algorithm is indicated by LEXICAL and can be omitted. Examples are:
CREATE TABLE t (a INT, b INT) PARTITIONED BY (c INT)
SORT BY ZORDER (a, b);
CREATE TABLE t SORT BY ZORDER (int_col,id) LIKE u;
CREATE TABLE t LIKE PARQUET '/foo' SORT BY ZORDER (id,zip);
ALTER TABLE t SORT BY ZORDER (int_col,id);
The following two are the same queries:
CREATE TABLE t (a INT, b INT) SORT BY (a, b);
CREATE TABLE t (a INT, b INT) SORT BY LEXICAL (a, b);
For strings, varchars, floats and doubles Z-ordering is not supported.
Currently ZORDER has the same functionality as a simple SORT BY clause,
therefore hidden behind a feature flag: unlock_zorder. The custom
sorting with Z-ordering will be in a different commit later.
Testing:
* Added tests for the ZORDER option for every SORT BY test.
* Modified some tests by adding the LEXICAL option.
* The .test workloads are temporarily put in separate test files
in order to set up the feature flag. These tests are run from
tests/custom_cluster/test_zorder.py which is a duplication of
the relevant tests, but with CustomClusterTestSuite decorator.
Change-Id: Ie122002ca8f52ca2c1e1ec8ff1d476ae1f4f875d
---
M be/src/common/global-flags.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M common/thrift/DataSinks.thrift
M common/thrift/JniCatalog.thrift
M common/thrift/PlanNodes.thrift
M common/thrift/Types.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSortByStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/SortInfo.java
M fe/src/main/java/org/apache/impala/analysis/TableDef.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java
M fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java
M fe/src/main/java/org/apache/impala/planner/ExchangeNode.java
M fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/planner/SortNode.java
M fe/src/main/java/org/apache/impala/planner/TableSink.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/jflex/sql-scanner.flex
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlUtilsTest.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/insert-sort-by-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/alter-table-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-as-select-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-like-file-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-like-table-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/show-create-table-zorder.test
M testdata/workloads/functional-query/queries/QueryTest/show-create-table.test
A tests/custom_cluster/test_zorder.py
44 files changed, 1,796 insertions(+), 149 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/55/13955/7
--
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: newpatchset
Gerrit-Change-Id: Ie122002ca8f52ca2c1e1ec8ff1d476ae1f4f875d
Gerrit-Change-Number: 13955
Gerrit-PatchSet: 7
Gerrit-Owner: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13955 )
Change subject: IMPALA-8755: Frontend support for Z-ordering
......................................................................
Patch Set 8:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/4232/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
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: 8
Gerrit-Owner: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 13 Aug 2019 09:54:48 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Posted by "Norbert Luksa (Code Review)" <ge...@cloudera.org>.
Norbert Luksa has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/13955 )
Change subject: IMPALA-8755: Frontend support for Z-ordering
......................................................................
IMPALA-8755: Frontend support for Z-ordering
Extended the SQL grammar with an optional flag for SORT BY, ZORDER.
If set, the new 'sort.algorithm' table property will be set to ZORDER
and the information will sink down to the backend. The default
algorithm is indicated by LEXICAL and can be omitted. Examples are:
CREATE TABLE t (a INT, b INT) PARTITIONED BY (c INT)
SORT BY ZORDER (a, b);
CREATE TABLE t SORT BY ZORDER (int_col,id) LIKE u;
CREATE TABLE t LIKE PARQUET '/foo' SORT BY ZORDER (id,zip);
ALTER TABLE t SORT BY ZORDER (int_col,id);
The following two are the same queries:
CREATE TABLE t (a INT, b INT) SORT BY (a, b);
CREATE TABLE t (a INT, b INT) SORT BY LEXICAL (a, b);
Currently ZORDER has the same functionality as a simple SORT BY clause,
therefore hidden behind a feature flag: unlock_zorder. The custom
sorting with Z-ordering will be in a different commit later.
Testing:
* Added tests for the ZORDER option for every SORT BY test.
* Modified some tests by adding the LEXICAL option.
* The .test workloads are temporarily put in separate test files
in order to set up the feature flag. These tests are run from
tests/custom_cluster/test_zorder.py which is a duplication of
the relevant tests, but with CustomClusterTestSuite decorator.
Change-Id: Ie122002ca8f52ca2c1e1ec8ff1d476ae1f4f875d
---
M be/src/common/global-flags.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M common/thrift/DataSinks.thrift
M common/thrift/JniCatalog.thrift
M common/thrift/PlanNodes.thrift
M common/thrift/Types.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSortByStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/SortInfo.java
M fe/src/main/java/org/apache/impala/analysis/TableDef.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java
M fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java
M fe/src/main/java/org/apache/impala/planner/ExchangeNode.java
M fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/planner/SortNode.java
M fe/src/main/java/org/apache/impala/planner/TableSink.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/jflex/sql-scanner.flex
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlUtilsTest.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/insert-sort-by-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/alter-table-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-as-select-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-like-file-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-like-table-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/show-create-table-zorder.test
M testdata/workloads/functional-query/queries/QueryTest/show-create-table.test
A tests/custom_cluster/test_zorder.py
44 files changed, 1,749 insertions(+), 146 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/55/13955/6
--
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: newpatchset
Gerrit-Change-Id: Ie122002ca8f52ca2c1e1ec8ff1d476ae1f4f875d
Gerrit-Change-Number: 13955
Gerrit-PatchSet: 6
Gerrit-Owner: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13955 )
Change subject: IMPALA-8755: Frontend support for Z-ordering
......................................................................
Patch Set 14:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/4639/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
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: 14
Gerrit-Owner: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 25 Sep 2019 11:30:41 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Posted by "Norbert Luksa (Code Review)" <ge...@cloudera.org>.
Norbert Luksa has uploaded a new patch set (#9). ( http://gerrit.cloudera.org:8080/13955 )
Change subject: IMPALA-8755: Frontend support for Z-ordering
......................................................................
IMPALA-8755: Frontend support for Z-ordering
Extended the SQL grammar with an optional flag for SORT BY, ZORDER.
If set, the new 'sort.algorithm' table property will be set to ZORDER
and the information will sink down to the backend. The default
order is indicated by LEXICAL and can be omitted. Examples are:
CREATE TABLE t (a INT, b INT) PARTITIONED BY (c INT)
SORT BY ZORDER (a, b);
CREATE TABLE t SORT BY ZORDER (int_col,id) LIKE u;
CREATE TABLE t LIKE PARQUET '/foo' SORT BY ZORDER (id,zip);
ALTER TABLE t SORT BY ZORDER (int_col,id);
The following two are the same statements:
CREATE TABLE t (a INT, b INT) SORT BY (a, b);
CREATE TABLE t (a INT, b INT) SORT BY LEXICAL (a, b);
For strings, varchars, floats and doubles Z-ordering is currently
not supported. It's not suitable for strings and varchars, but
support can be added for floats and doubles later.
Currently ZORDER has the same functionality as a simple SORT BY clause,
therefore hidden behind a feature flag: unlock_zorder. The custom
sorting with Z-ordering will be in a different commit later.
Testing:
* Added tests for the ZORDER option for every SORT BY test.
* Modified some tests by adding the LEXICAL option.
* The .test workloads are temporarily put in separate test files
in order to set up the feature flag. These tests are run from
tests/custom_cluster/test_zorder.py which is a duplication of
the relevant tests, but with CustomClusterTestSuite decorator.
Change-Id: Ie122002ca8f52ca2c1e1ec8ff1d476ae1f4f875d
---
M be/src/common/global-flags.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M common/thrift/DataSinks.thrift
M common/thrift/JniCatalog.thrift
M common/thrift/PlanNodes.thrift
M common/thrift/Types.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSortByStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/SortInfo.java
M fe/src/main/java/org/apache/impala/analysis/TableDef.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java
M fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java
M fe/src/main/java/org/apache/impala/planner/ExchangeNode.java
M fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/planner/SortNode.java
M fe/src/main/java/org/apache/impala/planner/TableSink.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/jflex/sql-scanner.flex
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlUtilsTest.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/insert-sort-by-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/alter-table-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-as-select-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-like-file-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-like-table-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/show-create-table-zorder.test
M testdata/workloads/functional-query/queries/QueryTest/show-create-table.test
A tests/custom_cluster/test_zorder.py
44 files changed, 1,796 insertions(+), 159 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/55/13955/9
--
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: newpatchset
Gerrit-Change-Id: Ie122002ca8f52ca2c1e1ec8ff1d476ae1f4f875d
Gerrit-Change-Number: 13955
Gerrit-PatchSet: 9
Gerrit-Owner: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
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 9: Code-Review+1
--
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: 9
Gerrit-Owner: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 14 Aug 2019 12:44:20 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13955 )
Change subject: IMPALA-8755: Frontend support for Z-ordering
......................................................................
Patch Set 11:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/4330/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
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: 11
Gerrit-Owner: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 22 Aug 2019 11:39:59 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Posted by "Norbert Luksa (Code Review)" <ge...@cloudera.org>.
Norbert Luksa has posted comments on this change. ( http://gerrit.cloudera.org:8080/13955 )
Change subject: IMPALA-8755: Frontend support for Z-ordering
......................................................................
Patch Set 12:
Thank you, Gábor, for the review and the advices.
--
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: 12
Gerrit-Owner: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 12 Sep 2019 13:37:47 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13955 )
Change subject: IMPALA-8755: Frontend support for Z-ordering
......................................................................
Patch Set 13:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/4638/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
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: 13
Gerrit-Owner: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 25 Sep 2019 10:36:17 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Posted by "Norbert Luksa (Code Review)" <ge...@cloudera.org>.
Norbert Luksa has uploaded a new patch set (#15). ( http://gerrit.cloudera.org:8080/13955 )
Change subject: IMPALA-8755: Frontend support for Z-ordering
......................................................................
IMPALA-8755: Frontend support for Z-ordering
Extended the SQL grammar with an optional and a default flag for
SORT BY, namely ZORDER and LEXICAL. If set, the new 'sort.algorithm'
table property will be set to ZORDER and the information will sink
down to the backend. The default order is indicated by LEXICAL
and can be omitted. Examples are:
CREATE TABLE t (a INT, b INT) PARTITIONED BY (c INT)
SORT BY ZORDER (a, b);
CREATE TABLE t SORT BY ZORDER (int_col,id) LIKE u;
CREATE TABLE t LIKE PARQUET '/foo' SORT BY ZORDER (id,zip);
ALTER TABLE t SORT BY ZORDER (int_col,id);
The following two are the same statements:
CREATE TABLE t (a INT, b INT) SORT BY (a, b);
CREATE TABLE t (a INT, b INT) SORT BY LEXICAL (a, b);
For strings, varchars, floats and doubles Z-ordering is currently
not supported. It's not suitable for strings and varchars, but
support can be added for floats and doubles later. The supported
types are: boolean, int types, decimals, date, timestamp, and char.
Currently ZORDER has the same functionality as a simple SORT BY clause,
therefore hidden behind a feature flag: unlock_zorder. The custom
sorting with Z-ordering will be in a different commit later.
Testing:
* Added tests for the ZORDER option for every SORT BY test.
* Modified some tests by adding the LEXICAL option.
* The .test workloads are temporarily put in separate test files
in order to set up the feature flag. These tests are run from
tests/custom_cluster/test_zorder.py which is a duplication of
the relevant tests, but with CustomClusterTestSuite decorator.
Change-Id: Ie122002ca8f52ca2c1e1ec8ff1d476ae1f4f875d
---
M be/src/common/global-flags.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M common/thrift/DataSinks.thrift
M common/thrift/JniCatalog.thrift
M common/thrift/PlanNodes.thrift
M common/thrift/Types.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSortByStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/SortInfo.java
M fe/src/main/java/org/apache/impala/analysis/TableDef.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java
M fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java
M fe/src/main/java/org/apache/impala/planner/ExchangeNode.java
M fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java
M fe/src/main/java/org/apache/impala/planner/PlanNode.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/planner/SortNode.java
M fe/src/main/java/org/apache/impala/planner/TableSink.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/jflex/sql-scanner.flex
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlUtilsTest.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/insert-sort-by-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/alter-table-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-as-select-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-like-file-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-like-table-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/show-create-table-zorder.test
M testdata/workloads/functional-query/queries/QueryTest/show-create-table.test
A tests/custom_cluster/test_zorder.py
45 files changed, 1,678 insertions(+), 171 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/55/13955/15
--
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: newpatchset
Gerrit-Change-Id: Ie122002ca8f52ca2c1e1ec8ff1d476ae1f4f875d
Gerrit-Change-Number: 13955
Gerrit-PatchSet: 15
Gerrit-Owner: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13955 )
Change subject: IMPALA-8755: Frontend support for Z-ordering
......................................................................
Patch Set 12:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13955/12/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/12/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java@1977
PS12, Line 1977: "SORT BY ZORDER does not support column types: STRING, VARCHAR(*), FLOAT, " +
line has trailing whitespace
--
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: 12
Gerrit-Owner: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 05 Sep 2019 10:30:40 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Posted by "Norbert Luksa (Code Review)" <ge...@cloudera.org>.
Norbert Luksa has posted comments on this change. ( http://gerrit.cloudera.org:8080/13955 )
Change subject: IMPALA-8755: Frontend support for Z-ordering
......................................................................
Patch Set 14:
(1 comment)
Fixed some minor things, removed some unnecessary test cases (duplicate of simple insert/sort scenarios).
http://gerrit.cloudera.org:8080/#/c/13955/14/fe/src/main/java/org/apache/impala/planner/TableSink.java
File fe/src/main/java/org/apache/impala/planner/TableSink.java:
http://gerrit.cloudera.org:8080/#/c/13955/14/fe/src/main/java/org/apache/impala/planner/TableSink.java@96
PS14, Line 96: sortColumns
> nit: comment outdated
Done
--
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: 14
Gerrit-Owner: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 26 Sep 2019 08:40:19 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Posted by "Norbert Luksa (Code Review)" <ge...@cloudera.org>.
Norbert Luksa has uploaded a new patch set (#11). ( http://gerrit.cloudera.org:8080/13955 )
Change subject: IMPALA-8755: Frontend support for Z-ordering
......................................................................
IMPALA-8755: Frontend support for Z-ordering
Extended the SQL grammar with an optional and a default flag for
SORT BY, namely ZORDER and LEXICAL. If set, the new 'sort.algorithm'
table property will be set to ZORDER and the information will sink
down to the backend. The default order is indicated by LEXICAL
and can be omitted. Examples are:
CREATE TABLE t (a INT, b INT) PARTITIONED BY (c INT)
SORT BY ZORDER (a, b);
CREATE TABLE t SORT BY ZORDER (int_col,id) LIKE u;
CREATE TABLE t LIKE PARQUET '/foo' SORT BY ZORDER (id,zip);
ALTER TABLE t SORT BY ZORDER (int_col,id);
The following two are the same statements:
CREATE TABLE t (a INT, b INT) SORT BY (a, b);
CREATE TABLE t (a INT, b INT) SORT BY LEXICAL (a, b);
For strings, varchars, floats and doubles Z-ordering is currently
not supported. It's not suitable for strings and varchars, but
support can be added for floats and doubles later. The supported
types are: boolean, int types, decimals, date, timestamp, and char.
Currently ZORDER has the same functionality as a simple SORT BY clause,
therefore hidden behind a feature flag: unlock_zorder. The custom
sorting with Z-ordering will be in a different commit later.
Testing:
* Added tests for the ZORDER option for every SORT BY test.
* Modified some tests by adding the LEXICAL option.
* The .test workloads are temporarily put in separate test files
in order to set up the feature flag. These tests are run from
tests/custom_cluster/test_zorder.py which is a duplication of
the relevant tests, but with CustomClusterTestSuite decorator.
Change-Id: Ie122002ca8f52ca2c1e1ec8ff1d476ae1f4f875d
---
M be/src/common/global-flags.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M common/thrift/DataSinks.thrift
M common/thrift/JniCatalog.thrift
M common/thrift/PlanNodes.thrift
M common/thrift/Types.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSortByStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/SortInfo.java
M fe/src/main/java/org/apache/impala/analysis/TableDef.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java
M fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java
M fe/src/main/java/org/apache/impala/planner/ExchangeNode.java
M fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java
M fe/src/main/java/org/apache/impala/planner/PlanNode.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/planner/SortNode.java
M fe/src/main/java/org/apache/impala/planner/TableSink.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/jflex/sql-scanner.flex
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlUtilsTest.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/insert-sort-by-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/alter-table-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-as-select-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-like-file-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-like-table-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/show-create-table-zorder.test
M testdata/workloads/functional-query/queries/QueryTest/show-create-table.test
A tests/custom_cluster/test_zorder.py
45 files changed, 1,792 insertions(+), 167 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/55/13955/11
--
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: newpatchset
Gerrit-Change-Id: Ie122002ca8f52ca2c1e1ec8ff1d476ae1f4f875d
Gerrit-Change-Number: 13955
Gerrit-PatchSet: 11
Gerrit-Owner: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13955 )
Change subject: IMPALA-8755: Frontend support for Z-ordering
......................................................................
Patch Set 9:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/4247/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
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: 9
Gerrit-Owner: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 14 Aug 2019 10:15:03 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
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 <no...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 09 Aug 2019 16:46:45 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13955 )
Change subject: IMPALA-8755: Frontend support for Z-ordering
......................................................................
Patch Set 15:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/4643/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
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: 15
Gerrit-Owner: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 26 Sep 2019 09:22:11 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Posted by "Norbert Luksa (Code Review)" <ge...@cloudera.org>.
Norbert Luksa has posted comments on this change. ( http://gerrit.cloudera.org:8080/13955 )
Change subject: IMPALA-8755: Frontend support for Z-ordering
......................................................................
Patch Set 9:
(14 comments)
Thank you for the feedback, added some comments, and changed some code.
http://gerrit.cloudera.org:8080/#/c/13955/9//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13955/9//COMMIT_MSG@9
PS9, Line 9: ,
> nit: is this comma needed?
Done
http://gerrit.cloudera.org:8080/#/c/13955/9//COMMIT_MSG@25
PS9, Line 25: For strings, varchars, floats and doubles Z-ordering is currently
: not supported.
> nit: Could you also state for which type(s) this is supported? Is it just I
Everything not mentioned are supported.
http://gerrit.cloudera.org:8080/#/c/13955/9//COMMIT_MSG@30
PS9, Line 30: unlock_zorder
> enable_zorder_sort or such seems better for me.
For me, 'unlock' sounds more like the word I'd use with an experimental feature, and 'enable' with some already implemented addition.
Maybe unlock_zorder_sort would be better?
http://gerrit.cloudera.org:8080/#/c/13955/9/fe/src/main/cup/sql-parser.cup
File fe/src/main/cup/sql-parser.cup:
http://gerrit.cloudera.org:8080/#/c/13955/9/fe/src/main/cup/sql-parser.cup@1399
PS9, Line 1399: CreateTableLikeStmt
> What if the source table used ZORDER sorting? Can "CREATE TABLE LIKE" state
The null passed previously was the the sorting columns, therefore in this case no sorting property was inherited. To add it again, someone has to add a SORT BY [ZORDER] clause, which will also define the ordering.
http://gerrit.cloudera.org:8080/#/c/13955/9/fe/src/main/cup/sql-parser.cup@1479
PS9, Line 1479: TSortingOrder.LEXICAL
> If there are no columns specified to sort by then does it make sense to say
NOT_APPLICABLE is also an option, since any new sorting setting would requires a SORT BY clause and therefore it would be set properly. However, for me, the default value or just null might sound better.
http://gerrit.cloudera.org:8080/#/c/13955/9/fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
File fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java:
http://gerrit.cloudera.org:8080/#/c/13955/9/fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java@50
PS9, Line 50: import com.google.common.collect.ImmutableList;
> Do you use this in this file?
Done
http://gerrit.cloudera.org:8080/#/c/13955/9/fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java@213
PS9, Line 213: throw new AnalysisException(String.format("'%s' or '%s' table property is not " +
: "supported for Kudu tables.", AlterTableSortByStmt.TBL_PROP_SORT_COLUMNS,
: AlterTableSortByStmt.TBL_PROP_SORT_ORDER));
> Not sure if this is something requested by someone, but I don't think exten
As far as I understand, this function is called when the 'sort.columns' or the 'sort.order' properties are set manually. (See at the comment before the class.)
We could write something like 'sort.*' properties are not supported for Kudu tables.
http://gerrit.cloudera.org:8080/#/c/13955/9/fe/src/main/java/org/apache/impala/analysis/TableDef.java
File fe/src/main/java/org/apache/impala/analysis/TableDef.java:
http://gerrit.cloudera.org:8080/#/c/13955/9/fe/src/main/java/org/apache/impala/analysis/TableDef.java@154
PS9, Line 154: this.sortingOrder = sortProperties.second;
> nit: Could you move this next to this.sortCols? Then both the usage of sort
Done
http://gerrit.cloudera.org:8080/#/c/13955/9/fe/src/main/java/org/apache/impala/analysis/TableDef.java@397
PS9, Line 397: throw new AnalysisException(String.format("SORT BY ZORDER does not support "
: + "column type: %s", colType.toSql()));
> What if multiple non-supported types are provided? Do we get an exception f
Yes, it raises an exception when finds the first one. Would it be better to scan all columns for unsupported types and return them all?
http://gerrit.cloudera.org:8080/#/c/13955/9/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
File fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java:
http://gerrit.cloudera.org:8080/#/c/13955/9/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@68
PS9, Line 68: ,e
> nit: missing space
Done
http://gerrit.cloudera.org:8080/#/c/13955/9/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@315
PS9, Line 315: TSortingOrder sortingOrder = TSortingOrder
> nit: This is needed only at L376, right? Makes sense to move it closer.
Most of the variables in this function are only needed in the return clause and not used anywhere else, so I'd keep it next to the other sorting related variable.
http://gerrit.cloudera.org:8080/#/c/13955/9/fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java
File fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java:
http://gerrit.cloudera.org:8080/#/c/13955/9/fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java@70
PS9, Line 70: sortingOrder
> nit: If 'sortingOrder_' is...
Done
http://gerrit.cloudera.org:8080/#/c/13955/9/fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java@70
PS9, Line 70: If sortingOrder is not lexicographical, the backend will skip this process.
> nit: Is this statement true until you deliver the BE implementation as well
The process is skipped, because when the order is not lexicographical, the parquet files won't be sorted in the usual manner, so the sortColumns_ variable won't actually store sorting columns (as parquet would expect them to be).
http://gerrit.cloudera.org:8080/#/c/13955/9/fe/src/main/java/org/apache/impala/planner/SortNode.java
File fe/src/main/java/org/apache/impala/planner/SortNode.java:
http://gerrit.cloudera.org:8080/#/c/13955/9/fe/src/main/java/org/apache/impala/planner/SortNode.java@211
PS9, Line 211: switch (info_.getSortingOrder()) {
: case LEXICAL:
: for (int i = 0; i < info_.getSortExprs().size(); ++i) {
: if (i > 0) output.append(", ");
: output.append(info_.getSortExprs().get(i).toSql() + " ");
: output.append(info_.getIsAscOrder().get(i) ? "ASC" : "DESC");
:
: Boolean nullsFirstParam = info_.getNullsFirstParams().get(i);
: if (nullsFirstParam != null) {
: output.append(nullsFirstParam ? " NULLS FIRST" : " NULLS LAST");
: }
: }
: break;
: case ZORDER:
: output.append("ZORDER: ");
: for (int i = 0; i < info_.getSortExprs().size(); ++i) {
: if (i > 0) output.append(", ");
: output.append(info_.getSortExprs().get(i).toSql());
: }
: break;
> This look very similar to what ExchangeNode.getNodeExplainString() does. Is
Merged the two in a function in the base class.
--
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: 9
Gerrit-Owner: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 22 Aug 2019 10:58:39 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/13955 )
Change subject: IMPALA-8755: Frontend support for Z-ordering
......................................................................
Patch Set 12: Code-Review+1
I'm fine with the changes but not competent enough to +2. Thanks for taking care of my comments!
Just a general comment: If you upload a patch set please make sure that it contains either your code changes or a rebase but not both. This makes it easier for reviewers to check the diffs between patch sets.
--
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: 12
Gerrit-Owner: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Sep 2019 09:04:44 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Posted by "Norbert Luksa (Code Review)" <ge...@cloudera.org>.
Norbert Luksa has posted comments on this change. ( http://gerrit.cloudera.org:8080/13955 )
Change subject: IMPALA-8755: Frontend support for Z-ordering
......................................................................
Patch Set 9:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/13955/11/fe/src/main/java/org/apache/impala/planner/PlanNode.java
File fe/src/main/java/org/apache/impala/planner/PlanNode.java:
http://gerrit.cloudera.org:8080/#/c/13955/11/fe/src/main/java/org/apache/impala/planner/PlanNode.java@641
PS11, Line 641: f stats-related varia
> nit: What about getSortingOrderExplainString() ?
Done
http://gerrit.cloudera.org:8080/#/c/13955/11/fe/src/test/java/org/apache/impala/analysis/ParserTest.java
File fe/src/test/java/org/apache/impala/analysis/ParserTest.java:
http://gerrit.cloudera.org:8080/#/c/13955/11/fe/src/test/java/org/apache/impala/analysis/ParserTest.java@2635
PS11, Line 2635: Baz
> nit: Baz? Isn't this meant to be "Bar"?
It should be correct with "Baz" (queries taken from the SORT BY tests)
http://gerrit.cloudera.org:8080/#/c/13955/11/tests/custom_cluster/test_zorder.py
File tests/custom_cluster/test_zorder.py:
http://gerrit.cloudera.org:8080/#/c/13955/11/tests/custom_cluster/test_zorder.py@29
PS11, Line 29: Temporary
> Could you explain why this is temporary? If this goes away when the rest of
Added extra comment with TODO, this test can be merged with other sort by tests when Z-ordering won't be hidden under a feature flag.
http://gerrit.cloudera.org:8080/#/c/13955/11/tests/custom_cluster/test_zorder.py@122
PS11, Line 122: Runs a show-create-table test file, containing the following sections:
:
: ---- CREATE_TABLE
: contains a table creation statement to create table TABLE_NAME
: ---- RESULTS
: contains the expected result of SHOW CREATE TABLE table_name
:
: OR
:
: ---- CREATE_VIEW
: contains a view creation statement to create table VIEW_NAME
: ---- RESULTS
: contains the expected result of SHOW CREATE VIEW table_name
:
: OR
:
: ---- QUERY
: a show create table query
: ---- RESULTS
: contains the expected output of the SHOW CREATE TABLE query
> nit: Could you re-phrase this function comment? I feel this is a bit overwh
The function and the comment was copied from the file mentioned above. Since it's only temporary, I would not leave it that way.
--
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: 9
Gerrit-Owner: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 03 Sep 2019 15:22:42 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Posted by "Norbert Luksa (Code Review)" <ge...@cloudera.org>.
Norbert Luksa has posted comments on this change. ( http://gerrit.cloudera.org:8080/13955 )
Change subject: IMPALA-8755: Frontend support for Z-ordering
......................................................................
Patch Set 6:
(14 comments)
http://gerrit.cloudera.org:8080/#/c/13955/5/fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java
File fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java:
http://gerrit.cloudera.org:8080/#/c/13955/5/fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java@58
PS5, Line 58: getRowFormat(), HdfsFileFormat.fromThrift(getFileFormat()), compression, null,
> line too long (102 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/13955/5/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/5/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java@2581
PS5, Line 2581: "tblproperties ('sort.algorithm'='ZORDER')", "Table definition must not " +
> line too long (91 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/13955/5/fe/src/test/java/org/apache/impala/analysis/ParserTest.java
File fe/src/test/java/org/apache/impala/analysis/ParserTest.java:
http://gerrit.cloudera.org:8080/#/c/13955/5/fe/src/test/java/org/apache/impala/analysis/ParserTest.java@2469
PS5, Line 2469: ParserError("ALTER TABLE TEST PARTITION (year=2009, month=4) SORT BY ZORDER " +
> line too long (96 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/13955/5/fe/src/test/java/org/apache/impala/analysis/ParserTest.java@2629
PS5, Line 2629: // Create table like other table with zsort columns
> line too long (91 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/13955/5/fe/src/test/java/org/apache/impala/planner/PlannerTest.java
File fe/src/test/java/org/apache/impala/planner/PlannerTest.java:
http://gerrit.cloudera.org:8080/#/c/13955/5/fe/src/test/java/org/apache/impala/planner/PlannerTest.java@262
PS5, Line 262: // Add a test table with a SORT BY ZORDER clause to test that the corresponding sort
> line too long (94 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/13955/5/tests/custom_cluster/test_zorder.py
File tests/custom_cluster/test_zorder.py:
http://gerrit.cloudera.org:8080/#/c/13955/5/tests/custom_cluster/test_zorder.py@18
PS5, Line 18: import shlex
> flake8: F401 'pytest' imported but unused
Done
http://gerrit.cloudera.org:8080/#/c/13955/5/tests/custom_cluster/test_zorder.py@22
PS5, Line 22: from tests.common.custom_cluster_test_suite import CustomClusterTestSuite
> flake8: F401 'copy.deepcopy' imported but unused
Done
http://gerrit.cloudera.org:8080/#/c/13955/5/tests/custom_cluster/test_zorder.py@33
PS5, Line 33: VALID_SECTION_NAMES = ["CREATE_TABLE", "CREATE_VIEW", "QUERY", "RESULTS"]
> flake8: E302 expected 2 blank lines, found 1
Done
http://gerrit.cloudera.org:8080/#/c/13955/5/tests/custom_cluster/test_zorder.py@78
PS5, Line 78:
> flake8: E501 line too long (97 > 90 characters)
Done
http://gerrit.cloudera.org:8080/#/c/13955/5/tests/custom_cluster/test_zorder.py@81
PS5, Line 81: @
> flake8: E303 too many blank lines (2)
Done
http://gerrit.cloudera.org:8080/#/c/13955/5/tests/custom_cluster/test_zorder.py@90
PS5, Line 90: # definition. The table is created, then the output of "SHOW CREATE TABLE" is used to
> flake8: E303 too many blank lines (3)
Done
http://gerrit.cloudera.org:8080/#/c/13955/5/tests/custom_cluster/test_zorder.py@94
PS5, Line 94: # Properties to filter before comparing results
> flake8: E302 expected 2 blank lines, found 3
Done
http://gerrit.cloudera.org:8080/#/c/13955/5/tests/custom_cluster/test_zorder.py@254
PS5, Line 254: RESULTS_DB_NAME_TOKEN = "show_create_table_test_db"
> flake8: E302 expected 2 blank lines, found 1
Done
http://gerrit.cloudera.org:8080/#/c/13955/5/tests/custom_cluster/test_zorder.py@302
PS5, Line 302:
> flake8: W391 blank line at end of file
Done
--
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 <no...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Aug 2019 12:37:16 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
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 14:
(1 comment)
Found a nit and PlannerTest.testInsertSortByZorder fails. Otherwise LGTM.
http://gerrit.cloudera.org:8080/#/c/13955/14/fe/src/main/java/org/apache/impala/planner/TableSink.java
File fe/src/main/java/org/apache/impala/planner/TableSink.java:
http://gerrit.cloudera.org:8080/#/c/13955/14/fe/src/main/java/org/apache/impala/planner/TableSink.java@96
PS14, Line 96: sortColumns
nit: comment outdated
--
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: 14
Gerrit-Owner: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 25 Sep 2019 14:11:25 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13955 )
Change subject: IMPALA-8755: Frontend support for Z-ordering
......................................................................
Patch Set 16: Code-Review+2
--
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: 16
Gerrit-Owner: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 26 Sep 2019 14:19:06 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13955 )
Change subject: IMPALA-8755: Frontend support for Z-ordering
......................................................................
Patch Set 6:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/4168/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
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 <no...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Aug 2019 13:18:35 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Posted by "Anurag Mantripragada (Code Review)" <ge...@cloudera.org>.
Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13955 )
Change subject: IMPALA-8755: Frontend support for Z-ordering
......................................................................
Patch Set 12: Code-Review+1
(1 comment)
Good work Norbert. In general the patch looks good to me. Please change the java docs for the methods that changed signatures or return types due to your patch. Apart from that, I see this has been reviewed already. I'm going to give my +1.
http://gerrit.cloudera.org:8080/#/c/13955/12/fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
File fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java:
http://gerrit.cloudera.org:8080/#/c/13955/12/fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java@206
PS12, Line 206: Returns a list of positions of the sort columns within the table's list of columns
Nit: Maybe change this statement to "Returns a Pair of list of positions of the sort columns within the table's list of columns and corresponding sort order" or something similar?
--
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: 12
Gerrit-Owner: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 17 Sep 2019 18:58:53 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13955 )
Change subject: IMPALA-8755: Frontend support for Z-ordering
......................................................................
Patch Set 16:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5004/ DRY_RUN=false
--
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: 16
Gerrit-Owner: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 26 Sep 2019 14:19:07 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
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 8:
(13 comments)
Had a couple of minor comments, nothing serious. Otherwise looks pretty good.
http://gerrit.cloudera.org:8080/#/c/13955/8//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13955/8//COMMIT_MSG@12
PS8, Line 12: algorithm
nit: order
http://gerrit.cloudera.org:8080/#/c/13955/8//COMMIT_MSG@21
PS8, Line 21: queries
statements
http://gerrit.cloudera.org:8080/#/c/13955/8//COMMIT_MSG@25
PS8, Line 25: For strings, varchars, floats and doubles Z-ordering is not supported.
You could mention that zorder is not suitable for string and varchars, but support for floats and doubles can be added later.
http://gerrit.cloudera.org:8080/#/c/13955/8/fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
File fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java:
http://gerrit.cloudera.org:8080/#/c/13955/8/fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java@221
PS8, Line 221: AlterTableSortByStmt.TBL_PROP_SORT_ORDER));
nit: missing spaces from the beginning
http://gerrit.cloudera.org:8080/#/c/13955/8/fe/src/main/java/org/apache/impala/analysis/AlterTableSortByStmt.java
File fe/src/main/java/org/apache/impala/analysis/AlterTableSortByStmt.java:
http://gerrit.cloudera.org:8080/#/c/13955/8/fe/src/main/java/org/apache/impala/analysis/AlterTableSortByStmt.java@37
PS8, Line 37: ZORDER
nit: ORDER, or LEXICAL|ZORDER
http://gerrit.cloudera.org:8080/#/c/13955/8/fe/src/main/java/org/apache/impala/analysis/AlterTableSortByStmt.java@65
PS8, Line 65:
nit: extra space
http://gerrit.cloudera.org:8080/#/c/13955/8/fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
File fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java:
http://gerrit.cloudera.org:8080/#/c/13955/8/fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java@51
PS8, Line 51: private final TSortingOrder sortingOrder_;
nit: you could place it after 'sortColumns_'
http://gerrit.cloudera.org:8080/#/c/13955/8/fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java@64
PS8, Line 64: List of
nit: Pair of ...
http://gerrit.cloudera.org:8080/#/c/13955/8/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
File fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java:
http://gerrit.cloudera.org:8080/#/c/13955/8/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java@113
PS8, Line 113: public TSortingOrder getSortingOrder() {
: return tableDef_.getSortingOrder();
: }
nit: fits single line
http://gerrit.cloudera.org:8080/#/c/13955/8/fe/src/main/java/org/apache/impala/analysis/SortInfo.java
File fe/src/main/java/org/apache/impala/analysis/SortInfo.java:
http://gerrit.cloudera.org:8080/#/c/13955/8/fe/src/main/java/org/apache/impala/analysis/SortInfo.java@62
PS8, Line 62: // Sort using Z-ordering order
nit: comment is obsolete
http://gerrit.cloudera.org:8080/#/c/13955/8/fe/src/main/java/org/apache/impala/analysis/SortInfo.java@103
PS8, Line 103: //if (sortingOrder_ == null) return TSortingOrder.LEXICAL;
nit: some old code has been left in the comment, please remove
http://gerrit.cloudera.org:8080/#/c/13955/8/testdata/workloads/functional-planner/queries/PlannerTest/insert-sort-by-zorder.test
File testdata/workloads/functional-planner/queries/PlannerTest/insert-sort-by-zorder.test:
http://gerrit.cloudera.org:8080/#/c/13955/8/testdata/workloads/functional-planner/queries/PlannerTest/insert-sort-by-zorder.test@424
PS8, Line 424: order by: ZORDER: id
are these tests still succeed? I think they should because we have a constant 'false' for 'bool_col' and it seems like a reasonable optimization.
http://gerrit.cloudera.org:8080/#/c/13955/8/testdata/workloads/functional-query/queries/QueryTest/create-table-zorder.test
File testdata/workloads/functional-query/queries/QueryTest/create-table-zorder.test:
http://gerrit.cloudera.org:8080/#/c/13955/8/testdata/workloads/functional-query/queries/QueryTest/create-table-zorder.test@3
PS8, Line 3: # Make sure that specifying sort by zorder columns sets the 'sort.columns'
nit: whitespace at eol
--
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: 8
Gerrit-Owner: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 13 Aug 2019 17:16:06 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13955 )
Change subject: IMPALA-8755: Frontend support for Z-ordering
......................................................................
Patch Set 12:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/4467/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
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: 12
Gerrit-Owner: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 05 Sep 2019 11:10:56 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Posted by "Norbert Luksa (Code Review)" <ge...@cloudera.org>.
Norbert Luksa has uploaded a new patch set (#14). ( http://gerrit.cloudera.org:8080/13955 )
Change subject: IMPALA-8755: Frontend support for Z-ordering
......................................................................
IMPALA-8755: Frontend support for Z-ordering
Extended the SQL grammar with an optional and a default flag for
SORT BY, namely ZORDER and LEXICAL. If set, the new 'sort.algorithm'
table property will be set to ZORDER and the information will sink
down to the backend. The default order is indicated by LEXICAL
and can be omitted. Examples are:
CREATE TABLE t (a INT, b INT) PARTITIONED BY (c INT)
SORT BY ZORDER (a, b);
CREATE TABLE t SORT BY ZORDER (int_col,id) LIKE u;
CREATE TABLE t LIKE PARQUET '/foo' SORT BY ZORDER (id,zip);
ALTER TABLE t SORT BY ZORDER (int_col,id);
The following two are the same statements:
CREATE TABLE t (a INT, b INT) SORT BY (a, b);
CREATE TABLE t (a INT, b INT) SORT BY LEXICAL (a, b);
For strings, varchars, floats and doubles Z-ordering is currently
not supported. It's not suitable for strings and varchars, but
support can be added for floats and doubles later. The supported
types are: boolean, int types, decimals, date, timestamp, and char.
Currently ZORDER has the same functionality as a simple SORT BY clause,
therefore hidden behind a feature flag: unlock_zorder. The custom
sorting with Z-ordering will be in a different commit later.
Testing:
* Added tests for the ZORDER option for every SORT BY test.
* Modified some tests by adding the LEXICAL option.
* The .test workloads are temporarily put in separate test files
in order to set up the feature flag. These tests are run from
tests/custom_cluster/test_zorder.py which is a duplication of
the relevant tests, but with CustomClusterTestSuite decorator.
Change-Id: Ie122002ca8f52ca2c1e1ec8ff1d476ae1f4f875d
---
M be/src/common/global-flags.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M common/thrift/DataSinks.thrift
M common/thrift/JniCatalog.thrift
M common/thrift/PlanNodes.thrift
M common/thrift/Types.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSortByStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/SortInfo.java
M fe/src/main/java/org/apache/impala/analysis/TableDef.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java
M fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java
M fe/src/main/java/org/apache/impala/planner/ExchangeNode.java
M fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java
M fe/src/main/java/org/apache/impala/planner/PlanNode.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/planner/SortNode.java
M fe/src/main/java/org/apache/impala/planner/TableSink.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/jflex/sql-scanner.flex
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlUtilsTest.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/insert-sort-by-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/alter-table-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-as-select-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-like-file-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-like-table-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/show-create-table-zorder.test
M testdata/workloads/functional-query/queries/QueryTest/show-create-table.test
A tests/custom_cluster/test_zorder.py
45 files changed, 1,816 insertions(+), 169 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/55/13955/14
--
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: newpatchset
Gerrit-Change-Id: Ie122002ca8f52ca2c1e1ec8ff1d476ae1f4f875d
Gerrit-Change-Number: 13955
Gerrit-PatchSet: 14
Gerrit-Owner: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13955 )
Change subject: IMPALA-8755: Frontend support for Z-ordering
......................................................................
IMPALA-8755: Frontend support for Z-ordering
Extended the SQL grammar with an optional and a default flag for
SORT BY, namely ZORDER and LEXICAL. If set, the new 'sort.algorithm'
table property will be set to ZORDER and the information will sink
down to the backend. The default order is indicated by LEXICAL
and can be omitted. Examples are:
CREATE TABLE t (a INT, b INT) PARTITIONED BY (c INT)
SORT BY ZORDER (a, b);
CREATE TABLE t SORT BY ZORDER (int_col,id) LIKE u;
CREATE TABLE t LIKE PARQUET '/foo' SORT BY ZORDER (id,zip);
ALTER TABLE t SORT BY ZORDER (int_col,id);
The following two are the same statements:
CREATE TABLE t (a INT, b INT) SORT BY (a, b);
CREATE TABLE t (a INT, b INT) SORT BY LEXICAL (a, b);
For strings, varchars, floats and doubles Z-ordering is currently
not supported. It's not suitable for strings and varchars, but
support can be added for floats and doubles later. The supported
types are: boolean, int types, decimals, date, timestamp, and char.
Currently ZORDER has the same functionality as a simple SORT BY clause,
therefore hidden behind a feature flag: unlock_zorder. The custom
sorting with Z-ordering will be in a different commit later.
Testing:
* Added tests for the ZORDER option for every SORT BY test.
* Modified some tests by adding the LEXICAL option.
* The .test workloads are temporarily put in separate test files
in order to set up the feature flag. These tests are run from
tests/custom_cluster/test_zorder.py which is a duplication of
the relevant tests, but with CustomClusterTestSuite decorator.
Change-Id: Ie122002ca8f52ca2c1e1ec8ff1d476ae1f4f875d
Reviewed-on: http://gerrit.cloudera.org:8080/13955
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/common/global-flags.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M common/thrift/DataSinks.thrift
M common/thrift/JniCatalog.thrift
M common/thrift/PlanNodes.thrift
M common/thrift/Types.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSortByStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/SortInfo.java
M fe/src/main/java/org/apache/impala/analysis/TableDef.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java
M fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java
M fe/src/main/java/org/apache/impala/planner/ExchangeNode.java
M fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java
M fe/src/main/java/org/apache/impala/planner/PlanNode.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/planner/SortNode.java
M fe/src/main/java/org/apache/impala/planner/TableSink.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/jflex/sql-scanner.flex
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlUtilsTest.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/insert-sort-by-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/alter-table-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-as-select-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-like-file-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-like-table-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/show-create-table-zorder.test
M testdata/workloads/functional-query/queries/QueryTest/show-create-table.test
A tests/custom_cluster/test_zorder.py
45 files changed, 1,678 insertions(+), 171 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
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: merged
Gerrit-Change-Id: Ie122002ca8f52ca2c1e1ec8ff1d476ae1f4f875d
Gerrit-Change-Number: 13955
Gerrit-PatchSet: 17
Gerrit-Owner: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Posted by "Norbert Luksa (Code Review)" <ge...@cloudera.org>.
Norbert Luksa has uploaded a new patch set (#13). ( http://gerrit.cloudera.org:8080/13955 )
Change subject: IMPALA-8755: Frontend support for Z-ordering
......................................................................
IMPALA-8755: Frontend support for Z-ordering
Extended the SQL grammar with an optional and a default flag for
SORT BY, namely ZORDER and LEXICAL. If set, the new 'sort.algorithm'
table property will be set to ZORDER and the information will sink
down to the backend. The default order is indicated by LEXICAL
and can be omitted. Examples are:
CREATE TABLE t (a INT, b INT) PARTITIONED BY (c INT)
SORT BY ZORDER (a, b);
CREATE TABLE t SORT BY ZORDER (int_col,id) LIKE u;
CREATE TABLE t LIKE PARQUET '/foo' SORT BY ZORDER (id,zip);
ALTER TABLE t SORT BY ZORDER (int_col,id);
The following two are the same statements:
CREATE TABLE t (a INT, b INT) SORT BY (a, b);
CREATE TABLE t (a INT, b INT) SORT BY LEXICAL (a, b);
For strings, varchars, floats and doubles Z-ordering is currently
not supported. It's not suitable for strings and varchars, but
support can be added for floats and doubles later. The supported
types are: boolean, int types, decimals, date, timestamp, and char.
Currently ZORDER has the same functionality as a simple SORT BY clause,
therefore hidden behind a feature flag: unlock_zorder. The custom
sorting with Z-ordering will be in a different commit later.
Testing:
* Added tests for the ZORDER option for every SORT BY test.
* Modified some tests by adding the LEXICAL option.
* The .test workloads are temporarily put in separate test files
in order to set up the feature flag. These tests are run from
tests/custom_cluster/test_zorder.py which is a duplication of
the relevant tests, but with CustomClusterTestSuite decorator.
Change-Id: Ie122002ca8f52ca2c1e1ec8ff1d476ae1f4f875d
---
M be/src/common/global-flags.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M common/thrift/DataSinks.thrift
M common/thrift/JniCatalog.thrift
M common/thrift/PlanNodes.thrift
M common/thrift/Types.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSortByStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/SortInfo.java
M fe/src/main/java/org/apache/impala/analysis/TableDef.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java
M fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java
M fe/src/main/java/org/apache/impala/planner/ExchangeNode.java
M fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java
M fe/src/main/java/org/apache/impala/planner/PlanNode.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/planner/SortNode.java
M fe/src/main/java/org/apache/impala/planner/TableSink.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/jflex/sql-scanner.flex
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlUtilsTest.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/insert-sort-by-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/alter-table-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-as-select-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-like-file-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-like-table-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/show-create-table-zorder.test
M testdata/workloads/functional-query/queries/QueryTest/show-create-table.test
A tests/custom_cluster/test_zorder.py
45 files changed, 1,801 insertions(+), 170 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/55/13955/13
--
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: newpatchset
Gerrit-Change-Id: Ie122002ca8f52ca2c1e1ec8ff1d476ae1f4f875d
Gerrit-Change-Number: 13955
Gerrit-PatchSet: 13
Gerrit-Owner: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13955 )
Change subject: IMPALA-8755: Frontend support for Z-ordering
......................................................................
Patch Set 16: Verified+1
--
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: 16
Gerrit-Owner: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 26 Sep 2019 18:35:04 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13955 )
Change subject: IMPALA-8755: Frontend support for Z-ordering
......................................................................
Patch Set 7:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/4221/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
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: 7
Gerrit-Owner: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 12 Aug 2019 17:48:22 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Posted by "Norbert Luksa (Code Review)" <ge...@cloudera.org>.
Norbert Luksa has posted comments on this change. ( http://gerrit.cloudera.org:8080/13955 )
Change subject: IMPALA-8755: Frontend support for Z-ordering
......................................................................
Patch Set 12:
(2 comments)
Thank you, Anurag! Changed the comments accordingly.
http://gerrit.cloudera.org:8080/#/c/13955/12/fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
File fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java:
http://gerrit.cloudera.org:8080/#/c/13955/12/fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java@206
PS12, Line 206: Returns a list of positions of the sort columns within the table's list of columns
> Nit: Maybe change this statement to "Returns a Pair of list of positions of
Done
http://gerrit.cloudera.org:8080/#/c/13955/12/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/12/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java@1977
PS12, Line 1977: "SORT BY ZORDER does not support column types: STRING, VARCHAR(*), FLOAT, " +
> line has trailing whitespace
Done
--
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: 12
Gerrit-Owner: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 25 Sep 2019 08:39:25 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Posted by "Norbert Luksa (Code Review)" <ge...@cloudera.org>.
Norbert Luksa 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. quick
You are right, replaced 'algorithm' with 'order'.
The Parquet stuff is relevant to the sort_columns parameter above, and I would leave it there for clarification.
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
Done
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
Done
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 prohi
Done
--
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 <no...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 12 Aug 2019 16:52:15 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
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 15: Code-Review+2
--
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: 15
Gerrit-Owner: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 26 Sep 2019 14:18:25 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Posted by "Norbert Luksa (Code Review)" <ge...@cloudera.org>.
Norbert Luksa has posted comments on this change. ( http://gerrit.cloudera.org:8080/13955 )
Change subject: IMPALA-8755: Frontend support for Z-ordering
......................................................................
Patch Set 11:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/13955/9//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13955/9//COMMIT_MSG@30
PS9, Line 30:
> hmm, indeed, unlock_zorder_sort would be slightly better. (nit)
Done
http://gerrit.cloudera.org:8080/#/c/13955/9/fe/src/main/java/org/apache/impala/analysis/TableDef.java
File fe/src/main/java/org/apache/impala/analysis/TableDef.java:
http://gerrit.cloudera.org:8080/#/c/13955/9/fe/src/main/java/org/apache/impala/analysis/TableDef.java@397
PS9, Line 397: throw new AnalysisException(String.format("SORT BY ZORDER does not support "
: + "column type: %s", colType.toSql()));
> Hmm, It might be more usable if you printed out all the unsupported columns
Sounds good, will change it that way.
--
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: 11
Gerrit-Owner: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 05 Sep 2019 10:29:47 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Posted by "Norbert Luksa (Code Review)" <ge...@cloudera.org>.
Norbert Luksa has uploaded a new patch set (#12). ( http://gerrit.cloudera.org:8080/13955 )
Change subject: IMPALA-8755: Frontend support for Z-ordering
......................................................................
IMPALA-8755: Frontend support for Z-ordering
Extended the SQL grammar with an optional and a default flag for
SORT BY, namely ZORDER and LEXICAL. If set, the new 'sort.algorithm'
table property will be set to ZORDER and the information will sink
down to the backend. The default order is indicated by LEXICAL
and can be omitted. Examples are:
CREATE TABLE t (a INT, b INT) PARTITIONED BY (c INT)
SORT BY ZORDER (a, b);
CREATE TABLE t SORT BY ZORDER (int_col,id) LIKE u;
CREATE TABLE t LIKE PARQUET '/foo' SORT BY ZORDER (id,zip);
ALTER TABLE t SORT BY ZORDER (int_col,id);
The following two are the same statements:
CREATE TABLE t (a INT, b INT) SORT BY (a, b);
CREATE TABLE t (a INT, b INT) SORT BY LEXICAL (a, b);
For strings, varchars, floats and doubles Z-ordering is currently
not supported. It's not suitable for strings and varchars, but
support can be added for floats and doubles later. The supported
types are: boolean, int types, decimals, date, timestamp, and char.
Currently ZORDER has the same functionality as a simple SORT BY clause,
therefore hidden behind a feature flag: unlock_zorder. The custom
sorting with Z-ordering will be in a different commit later.
Testing:
* Added tests for the ZORDER option for every SORT BY test.
* Modified some tests by adding the LEXICAL option.
* The .test workloads are temporarily put in separate test files
in order to set up the feature flag. These tests are run from
tests/custom_cluster/test_zorder.py which is a duplication of
the relevant tests, but with CustomClusterTestSuite decorator.
Change-Id: Ie122002ca8f52ca2c1e1ec8ff1d476ae1f4f875d
---
M be/src/common/global-flags.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M common/thrift/DataSinks.thrift
M common/thrift/JniCatalog.thrift
M common/thrift/PlanNodes.thrift
M common/thrift/Types.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSortByStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/SortInfo.java
M fe/src/main/java/org/apache/impala/analysis/TableDef.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java
M fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java
M fe/src/main/java/org/apache/impala/planner/ExchangeNode.java
M fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java
M fe/src/main/java/org/apache/impala/planner/PlanNode.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/planner/SortNode.java
M fe/src/main/java/org/apache/impala/planner/TableSink.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/jflex/sql-scanner.flex
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlUtilsTest.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/insert-sort-by-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/alter-table-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-as-select-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-like-file-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-like-table-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/show-create-table-zorder.test
M testdata/workloads/functional-query/queries/QueryTest/show-create-table.test
A tests/custom_cluster/test_zorder.py
45 files changed, 1,796 insertions(+), 166 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/55/13955/12
--
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: newpatchset
Gerrit-Change-Id: Ie122002ca8f52ca2c1e1ec8ff1d476ae1f4f875d
Gerrit-Change-Number: 13955
Gerrit-PatchSet: 12
Gerrit-Owner: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Posted by "Norbert Luksa (Code Review)" <ge...@cloudera.org>.
Norbert Luksa has uploaded a new patch set (#8). ( http://gerrit.cloudera.org:8080/13955 )
Change subject: IMPALA-8755: Frontend support for Z-ordering
......................................................................
IMPALA-8755: Frontend support for Z-ordering
Extended the SQL grammar with an optional flag for SORT BY, ZORDER.
If set, the new 'sort.algorithm' table property will be set to ZORDER
and the information will sink down to the backend. The default
algorithm is indicated by LEXICAL and can be omitted. Examples are:
CREATE TABLE t (a INT, b INT) PARTITIONED BY (c INT)
SORT BY ZORDER (a, b);
CREATE TABLE t SORT BY ZORDER (int_col,id) LIKE u;
CREATE TABLE t LIKE PARQUET '/foo' SORT BY ZORDER (id,zip);
ALTER TABLE t SORT BY ZORDER (int_col,id);
The following two are the same queries:
CREATE TABLE t (a INT, b INT) SORT BY (a, b);
CREATE TABLE t (a INT, b INT) SORT BY LEXICAL (a, b);
For strings, varchars, floats and doubles Z-ordering is not supported.
Currently ZORDER has the same functionality as a simple SORT BY clause,
therefore hidden behind a feature flag: unlock_zorder. The custom
sorting with Z-ordering will be in a different commit later.
Testing:
* Added tests for the ZORDER option for every SORT BY test.
* Modified some tests by adding the LEXICAL option.
* The .test workloads are temporarily put in separate test files
in order to set up the feature flag. These tests are run from
tests/custom_cluster/test_zorder.py which is a duplication of
the relevant tests, but with CustomClusterTestSuite decorator.
Change-Id: Ie122002ca8f52ca2c1e1ec8ff1d476ae1f4f875d
---
M be/src/common/global-flags.cc
M be/src/util/backend-gflag-util.cc
M common/thrift/BackendGflags.thrift
M common/thrift/DataSinks.thrift
M common/thrift/JniCatalog.thrift
M common/thrift/PlanNodes.thrift
M common/thrift/Types.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableSortByStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/SortInfo.java
M fe/src/main/java/org/apache/impala/analysis/TableDef.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java
M fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java
M fe/src/main/java/org/apache/impala/planner/ExchangeNode.java
M fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/planner/SortNode.java
M fe/src/main/java/org/apache/impala/planner/TableSink.java
M fe/src/main/java/org/apache/impala/service/BackendConfig.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/jflex/sql-scanner.flex
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlUtilsTest.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/insert-sort-by-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/alter-table-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-as-select-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-like-file-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-like-table-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/create-table-zorder.test
A testdata/workloads/functional-query/queries/QueryTest/show-create-table-zorder.test
M testdata/workloads/functional-query/queries/QueryTest/show-create-table.test
A tests/custom_cluster/test_zorder.py
44 files changed, 1,794 insertions(+), 149 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/55/13955/8
--
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: newpatchset
Gerrit-Change-Id: Ie122002ca8f52ca2c1e1ec8ff1d476ae1f4f875d
Gerrit-Change-Number: 13955
Gerrit-PatchSet: 8
Gerrit-Owner: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/13955 )
Change subject: IMPALA-8755: Frontend support for Z-ordering
......................................................................
Patch Set 11:
Have you forgot to upload the latest patch set? :)
--
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: 11
Gerrit-Owner: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 04 Sep 2019 06:44:04 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Posted by "Norbert Luksa (Code Review)" <ge...@cloudera.org>.
Norbert Luksa has posted comments on this change. ( http://gerrit.cloudera.org:8080/13955 )
Change subject: IMPALA-8755: Frontend support for Z-ordering
......................................................................
Patch Set 9:
(13 comments)
http://gerrit.cloudera.org:8080/#/c/13955/8//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13955/8//COMMIT_MSG@12
PS8, Line 12: order is
> nit: order
Done
http://gerrit.cloudera.org:8080/#/c/13955/8//COMMIT_MSG@21
PS8, Line 21: stateme
> statements
Done
http://gerrit.cloudera.org:8080/#/c/13955/8//COMMIT_MSG@25
PS8, Line 25: For strings, varchars, floats and doubles Z-ordering is currently
> You could mention that zorder is not suitable for string and varchars, but
Done
http://gerrit.cloudera.org:8080/#/c/13955/8/fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java
File fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java:
http://gerrit.cloudera.org:8080/#/c/13955/8/fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java@221
PS8, Line 221: AlterTableSortByStmt.TBL_PROP_SORT_ORDER));
> nit: missing spaces from the beginning
Done
http://gerrit.cloudera.org:8080/#/c/13955/8/fe/src/main/java/org/apache/impala/analysis/AlterTableSortByStmt.java
File fe/src/main/java/org/apache/impala/analysis/AlterTableSortByStmt.java:
http://gerrit.cloudera.org:8080/#/c/13955/8/fe/src/main/java/org/apache/impala/analysis/AlterTableSortByStmt.java@37
PS8, Line 37: LEXICA
> nit: ORDER, or LEXICAL|ZORDER
Done
http://gerrit.cloudera.org:8080/#/c/13955/8/fe/src/main/java/org/apache/impala/analysis/AlterTableSortByStmt.java@65
PS8, Line 65:
> nit: extra space
Done
http://gerrit.cloudera.org:8080/#/c/13955/8/fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
File fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java:
http://gerrit.cloudera.org:8080/#/c/13955/8/fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java@51
PS8, Line 51: private final boolean ifNotExists_;
> nit: you could place it after 'sortColumns_'
Done
http://gerrit.cloudera.org:8080/#/c/13955/8/fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java@64
PS8, Line 64: A pair,
> nit: Pair of ...
Done
http://gerrit.cloudera.org:8080/#/c/13955/8/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
File fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java:
http://gerrit.cloudera.org:8080/#/c/13955/8/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java@113
PS8, Line 113: public String getComment() { return tableDef_.getComment(); }
: Map<String, String> getTblProperties() { return tableDef_.getTblProperties(); }
: p
> nit: fits single line
Done
http://gerrit.cloudera.org:8080/#/c/13955/8/fe/src/main/java/org/apache/impala/analysis/SortInfo.java
File fe/src/main/java/org/apache/impala/analysis/SortInfo.java:
http://gerrit.cloudera.org:8080/#/c/13955/8/fe/src/main/java/org/apache/impala/analysis/SortInfo.java@62
PS8, Line 62: private TSortingOrder sortingO
> nit: comment is obsolete
Done
http://gerrit.cloudera.org:8080/#/c/13955/8/fe/src/main/java/org/apache/impala/analysis/SortInfo.java@103
PS8, Line 103: * Gets the list of booleans indicating whether nulls come f
> nit: some old code has been left in the comment, please remove
Done
http://gerrit.cloudera.org:8080/#/c/13955/8/testdata/workloads/functional-planner/queries/PlannerTest/insert-sort-by-zorder.test
File testdata/workloads/functional-planner/queries/PlannerTest/insert-sort-by-zorder.test:
http://gerrit.cloudera.org:8080/#/c/13955/8/testdata/workloads/functional-planner/queries/PlannerTest/insert-sort-by-zorder.test@424
PS8, Line 424: order by: ZORDER: id
> are these tests still succeed? I think they should because we have a consta
yes, the test succeeds, the optimisation worked without any additional change
http://gerrit.cloudera.org:8080/#/c/13955/8/testdata/workloads/functional-query/queries/QueryTest/create-table-zorder.test
File testdata/workloads/functional-query/queries/QueryTest/create-table-zorder.test:
http://gerrit.cloudera.org:8080/#/c/13955/8/testdata/workloads/functional-query/queries/QueryTest/create-table-zorder.test@3
PS8, Line 3: # Make sure that specifying sort by zorder columns sets the 'sort.columns'
> nit: whitespace at eol
Done
--
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: 9
Gerrit-Owner: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 14 Aug 2019 09:33:51 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13955 )
Change subject: IMPALA-8755: Frontend support for Z-ordering
......................................................................
Patch Set 5:
(14 comments)
http://gerrit.cloudera.org:8080/#/c/13955/5/fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java
File fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java:
http://gerrit.cloudera.org:8080/#/c/13955/5/fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java@58
PS5, Line 58: getRowFormat(), HdfsFileFormat.fromThrift(getFileFormat()), compression, null, getLocation());
line too long (102 > 90)
http://gerrit.cloudera.org:8080/#/c/13955/5/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/5/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java@2581
PS5, Line 2581: "tblproperties ('sort.algorithm'='ZORDER')", "Table definition must not contain " +
line too long (91 > 90)
http://gerrit.cloudera.org:8080/#/c/13955/5/fe/src/test/java/org/apache/impala/analysis/ParserTest.java
File fe/src/test/java/org/apache/impala/analysis/ParserTest.java:
http://gerrit.cloudera.org:8080/#/c/13955/5/fe/src/test/java/org/apache/impala/analysis/ParserTest.java@2469
PS5, Line 2469: ParserError("ALTER TABLE TEST PARTITION (year=2009, month=4) SORT BY ZORDER (int_col, id)");
line too long (96 > 90)
http://gerrit.cloudera.org:8080/#/c/13955/5/fe/src/test/java/org/apache/impala/analysis/ParserTest.java@2629
PS5, Line 2629: ParsesOk("CREATE TABLE Foo SORT BY ZORDER(bar) LIKE Baz STORED AS TEXTFILE LOCATION " +
line too long (91 > 90)
http://gerrit.cloudera.org:8080/#/c/13955/5/fe/src/test/java/org/apache/impala/planner/PlannerTest.java
File fe/src/test/java/org/apache/impala/planner/PlannerTest.java:
http://gerrit.cloudera.org:8080/#/c/13955/5/fe/src/test/java/org/apache/impala/planner/PlannerTest.java@262
PS5, Line 262: // Add a test table with a SORT BY ZORDER clause to test that the corresponding sort nodes
line too long (94 > 90)
http://gerrit.cloudera.org:8080/#/c/13955/5/tests/custom_cluster/test_zorder.py
File tests/custom_cluster/test_zorder.py:
http://gerrit.cloudera.org:8080/#/c/13955/5/tests/custom_cluster/test_zorder.py@18
PS5, Line 18: import pytest
flake8: F401 'pytest' imported but unused
http://gerrit.cloudera.org:8080/#/c/13955/5/tests/custom_cluster/test_zorder.py@22
PS5, Line 22: from copy import deepcopy
flake8: F401 'copy.deepcopy' imported but unused
http://gerrit.cloudera.org:8080/#/c/13955/5/tests/custom_cluster/test_zorder.py@33
PS5, Line 33: class TestZOrder(CustomClusterTestSuite):
flake8: E302 expected 2 blank lines, found 1
http://gerrit.cloudera.org:8080/#/c/13955/5/tests/custom_cluster/test_zorder.py@78
PS5, Line 78: t
flake8: E501 line too long (97 > 90 characters)
http://gerrit.cloudera.org:8080/#/c/13955/5/tests/custom_cluster/test_zorder.py@81
PS5, Line 81: @
flake8: E303 too many blank lines (2)
http://gerrit.cloudera.org:8080/#/c/13955/5/tests/custom_cluster/test_zorder.py@90
PS5, Line 90: # The purpose of the show create table tests are to ensure that the "SHOW CREATE TABLE"
flake8: E303 too many blank lines (3)
http://gerrit.cloudera.org:8080/#/c/13955/5/tests/custom_cluster/test_zorder.py@94
PS5, Line 94: class TestZOrderShowCreateTable(CustomClusterTestSuite):
flake8: E302 expected 2 blank lines, found 3
http://gerrit.cloudera.org:8080/#/c/13955/5/tests/custom_cluster/test_zorder.py@254
PS5, Line 254: class ShowCreateTableTestCase(object):
flake8: E302 expected 2 blank lines, found 1
http://gerrit.cloudera.org:8080/#/c/13955/5/tests/custom_cluster/test_zorder.py@302
PS5, Line 302:
flake8: W391 blank line at end of file
--
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: 5
Gerrit-Owner: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Aug 2019 08:26:15 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13955 )
Change subject: IMPALA-8755: Frontend support for Z-ordering
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/4148/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
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: 5
Gerrit-Owner: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Aug 2019 09:06:28 +0000
Gerrit-HasComments: No