You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Fredy Wijaya (Code Review)" <ge...@cloudera.org> on 2019/02/24 00:05:37 UTC
[Impala-ASF-CR] IMPALA-7645: Add a query option to set default table file format
Fredy Wijaya has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12568
Change subject: IMPALA-7645: Add a query option to set default table file format
......................................................................
IMPALA-7645: Add a query option to set default table file format
This patch adds a query option "TABLE_FILE_FORMAT" to allow setting the
default table file format. Below is the list of supported
"TABLE_FILE_FORMAT" query option values:
- TEXT
- RC_FILE
- SEQUENCE_FILE
- AVRO
- PARQUET
- KUDU
- ORC
For backward compatibility, the default table file format remains as
TEXT.
Testing:
- Ran all FE tests
- Added BE test
- Added E2E tests
- Ran test_ddl.py and test_set.py
Change-Id: Ic857c38076d973ad749a41fecd1b470c7881db5e
---
M be/src/service/query-options-test.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/TableDef.java
M testdata/workloads/functional-query/queries/QueryTest/set.test
M tests/metadata/test_ddl.py
9 files changed, 95 insertions(+), 9 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/68/12568/3
--
To view, visit http://gerrit.cloudera.org:8080/12568
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic857c38076d973ad749a41fecd1b470c7881db5e
Gerrit-Change-Number: 12568
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
[Impala-ASF-CR] IMPALA-7645: Add a query option to set the default table file format
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12568 )
Change subject: IMPALA-7645: Add a query option to set the default table file format
......................................................................
Patch Set 7:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2249/ : 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/12568
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic857c38076d973ad749a41fecd1b470c7881db5e
Gerrit-Change-Number: 12568
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 26 Feb 2019 05:24:21 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7645: Add a query option to set the default table file format
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12568 )
Change subject: IMPALA-7645: Add a query option to set the default table file format
......................................................................
Patch Set 7: Code-Review+2
(2 comments)
http://gerrit.cloudera.org:8080/#/c/12568/5/be/src/service/query-options.cc
File be/src/service/query-options.cc:
http://gerrit.cloudera.org:8080/#/c/12568/5/be/src/service/query-options.cc@785
PS5, Line 785: TEXT, RC_FILE, SEQUENCE_FILE, AVRO, PARQUET, KUDU "
: "and ORC.
> My thinking is every time we add a new file format, we will definitely need
wfm
http://gerrit.cloudera.org:8080/#/c/12568/5/fe/src/main/cup/sql-parser.cup
File fe/src/main/cup/sql-parser.cup:
http://gerrit.cloudera.org:8080/#/c/12568/5/fe/src/main/cup/sql-parser.cup@1322
PS5, Line 1322: comment, parser.getQueryOptions()
> The syntax for CREATE TABLE <table> PRODUCED BY DATA SOURCE <data source> d
Fair point
--
To view, visit http://gerrit.cloudera.org:8080/12568
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic857c38076d973ad749a41fecd1b470c7881db5e
Gerrit-Change-Number: 12568
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 27 Feb 2019 03:34:21 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7645: Add a query option to set the default table file format
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12568 )
Change subject: IMPALA-7645: Add a query option to set the default table file format
......................................................................
Patch Set 6:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2248/ : 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/12568
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic857c38076d973ad749a41fecd1b470c7881db5e
Gerrit-Change-Number: 12568
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 26 Feb 2019 04:06:35 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7645: Add a query option to set the default table file format
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12568 )
Change subject: IMPALA-7645: Add a query option to set the default table file format
......................................................................
Patch Set 6:
> Patch Set 6:
>
> Can the file format be set in the sql statement itself? I always consider it a bit of a hack to have to set a query option before running a statement: just one more thing for the user to remember. And, to remember to set it back afterward.
We can use "set". It's not technically an SQL statement because of the way we implement it (set is a shell thing)
set DEFAULT_FILE_FORMAT=parquet
We can also unset it. Is that good enough?
--
To view, visit http://gerrit.cloudera.org:8080/12568
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic857c38076d973ad749a41fecd1b470c7881db5e
Gerrit-Change-Number: 12568
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 26 Feb 2019 03:50:55 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7645: Add a query option to set the default table file format
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12568 )
Change subject: IMPALA-7645: Add a query option to set the default table file format
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2225/ : 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/12568
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic857c38076d973ad749a41fecd1b470c7881db5e
Gerrit-Change-Number: 12568
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Sun, 24 Feb 2019 00:49:49 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7645: Add a query option to set the default table file format
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/12568 )
Change subject: IMPALA-7645: Add a query option to set the default table file format
......................................................................
IMPALA-7645: Add a query option to set the default table file format
This patch adds a query option "DEFAULT_FILE_FORMAT" to allow setting
the default table file format. Below is the list of supported
"DEFAULT_FILE_FORMAT" query option values:
- TEXT
- RC_FILE
- SEQUENCE_FILE
- AVRO
- PARQUET
- KUDU
- ORC
For backward compatibility, the default table file format remains as
TEXT.
Testing:
- Ran all FE tests
- Added BE test
- Added E2E tests
- Ran test_ddl.py and test_set.py
Change-Id: Ic857c38076d973ad749a41fecd1b470c7881db5e
---
M be/src/service/query-options-test.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/TableDef.java
M testdata/workloads/functional-query/queries/QueryTest/set.test
M tests/metadata/test_ddl.py
9 files changed, 95 insertions(+), 9 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/68/12568/6
--
To view, visit http://gerrit.cloudera.org:8080/12568
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic857c38076d973ad749a41fecd1b470c7881db5e
Gerrit-Change-Number: 12568
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-7645: Add a query option to set the default table file format
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/12568 )
Change subject: IMPALA-7645: Add a query option to set the default table file format
......................................................................
IMPALA-7645: Add a query option to set the default table file format
This patch adds a query option "DEFAULT_FILE_FORMAT" to allow setting
the default table file format. Below is the list of supported
"DEFAULT_FILE_FORMAT" query option values:
- TEXT
- RC_FILE
- SEQUENCE_FILE
- AVRO
- PARQUET
- KUDU
- ORC
For backward compatibility, the default table file format remains as
TEXT.
Testing:
- Ran all FE tests
- Added BE test
- Added E2E tests
- Ran test_ddl.py and test_set.py
Change-Id: Ic857c38076d973ad749a41fecd1b470c7881db5e
Reviewed-on: http://gerrit.cloudera.org:8080/12568
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/service/query-options-test.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/TableDef.java
M testdata/workloads/functional-query/queries/QueryTest/set.test
M tests/metadata/test_ddl.py
9 files changed, 92 insertions(+), 9 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/12568
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic857c38076d973ad749a41fecd1b470c7881db5e
Gerrit-Change-Number: 12568
Gerrit-PatchSet: 9
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-7645: Add a query option to set the default table file format
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12568 )
Change subject: IMPALA-7645: Add a query option to set the default table file format
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2226/ : 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/12568
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic857c38076d973ad749a41fecd1b470c7881db5e
Gerrit-Change-Number: 12568
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Sun, 24 Feb 2019 00:51:06 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7645: Add a query option to set the default table file format
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12568 )
Change subject: IMPALA-7645: Add a query option to set the default table file format
......................................................................
Patch Set 8:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3840/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/12568
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic857c38076d973ad749a41fecd1b470c7881db5e
Gerrit-Change-Number: 12568
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 27 Feb 2019 03:37:10 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7645: Add a query option to set the default table file format
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12568 )
Change subject: IMPALA-7645: Add a query option to set the default table file format
......................................................................
Patch Set 7:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/12568/5/be/src/service/query-options.cc
File be/src/service/query-options.cc:
http://gerrit.cloudera.org:8080/#/c/12568/5/be/src/service/query-options.cc@785
PS5, Line 785: TEXT, RC_FILE, SEQUENCE_FILE, AVRO, PARQUET, KUDU "
: "and ORC.
> nit: will probably become stale.
My thinking is every time we add a new file format, we will definitely need to update this file because of the if/else logic. So technically we should never miss updating the error message. It's also nice to know which values are allowed since we aren't very consistent in matching the enum names vs the SQL keywords, e.g.
TEXTFILE (SQL) vs TEXT (enum)
RCFILE (SQL) vs RC_FILE (enum)
etc.
Let me know what you think.
http://gerrit.cloudera.org:8080/#/c/12568/5/common/thrift/ImpalaInternalService.thrift
File common/thrift/ImpalaInternalService.thrift:
http://gerrit.cloudera.org:8080/#/c/12568/5/common/thrift/ImpalaInternalService.thrift@328
PS5, Line 328: at
> Can we just set the default here? default_file_format = TEXT
Good idea. Done.
http://gerrit.cloudera.org:8080/#/c/12568/5/fe/src/main/cup/sql-parser.cup
File fe/src/main/cup/sql-parser.cup:
http://gerrit.cloudera.org:8080/#/c/12568/5/fe/src/main/cup/sql-parser.cup@1322
PS5, Line 1322: comment, parser.getQueryOptions()
> Also, how about moving this into tbl_options? There we can resolve the effe
The syntax for CREATE TABLE <table> PRODUCED BY DATA SOURCE <data source> doesn't support tbl_options. The only thing it supports from tbl_options is comment. That's probably why it's slightly different than regular CREATE TABLE.
http://gerrit.cloudera.org:8080/#/c/12568/5/fe/src/main/cup/sql-parser.cup@1322
PS5, Line 1322: parser.getQueryOptions(
> Do we need to pass the whole opts list? Seems kinda weird that TableDef has
No required, but it's actually nice if in the future we need to get more things from query options. We no longer need to update the parser. Similar example: https://github.com/apache/impala/blob/master/fe/src/main/cup/sql-parser.cup#L3014-L3020
http://gerrit.cloudera.org:8080/#/c/12568/5/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/12568/5/fe/src/main/java/org/apache/impala/analysis/TableDef.java@137
PS5, Line 137: / The file format passed via STORED AS <file format> has a higher precedence than
: // the one set in query options.
: this.fileFormat = (fileFormat != null) ?
: fileFormat : queryOptions.getDefault_file_format();
: this.location = location;
: this.cachingOp = cachingOp;
: Preconditions.checkNotNull(tblProperties);
: t
> I think we can move all of this into the parser itself. Please refer to my
I'm actually not a big fan of putting too much logic in the parser. It can make the code somewhat unreadable.
--
To view, visit http://gerrit.cloudera.org:8080/12568
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic857c38076d973ad749a41fecd1b470c7881db5e
Gerrit-Change-Number: 12568
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 26 Feb 2019 05:00:28 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7645: Add a query option to set the default table file format
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12568 )
Change subject: IMPALA-7645: Add a query option to set the default table file format
......................................................................
Patch Set 4: Code-Review+1
(1 comment)
LGTM. Had one thought about naming but I think I'm fine with this.
http://gerrit.cloudera.org:8080/#/c/12568/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/12568/4//COMMIT_MSG@9
PS4, Line 9: TABLE_FILE_FORMAT
Maybe DEFAULT_FILE_FORMAT? Only because people might be confused about whether it overrides the "STORED AS".
Actually I guess they probably won't be.
--
To view, visit http://gerrit.cloudera.org:8080/12568
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic857c38076d973ad749a41fecd1b470c7881db5e
Gerrit-Change-Number: 12568
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 25 Feb 2019 21:19:08 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7645: Add a query option to set the default table file format
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/12568 )
Change subject: IMPALA-7645: Add a query option to set the default table file format
......................................................................
IMPALA-7645: Add a query option to set the default table file format
This patch adds a query option "DEFAULT_FILE_FORMAT" to allow setting
the default table file format. Below is the list of supported
"DEFAULT_FILE_FORMAT" query option values:
- TEXT
- RC_FILE
- SEQUENCE_FILE
- AVRO
- PARQUET
- KUDU
- ORC
For backward compatibility, the default table file format remains as
TEXT.
Testing:
- Ran all FE tests
- Added BE test
- Added E2E tests
- Ran test_ddl.py and test_set.py
Change-Id: Ic857c38076d973ad749a41fecd1b470c7881db5e
---
M be/src/service/query-options-test.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/TableDef.java
M testdata/workloads/functional-query/queries/QueryTest/set.test
M tests/metadata/test_ddl.py
9 files changed, 92 insertions(+), 9 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/68/12568/7
--
To view, visit http://gerrit.cloudera.org:8080/12568
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic857c38076d973ad749a41fecd1b470c7881db5e
Gerrit-Change-Number: 12568
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-7645: Add a query option to set the default table file format
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12568 )
Change subject: IMPALA-7645: Add a query option to set the default table file format
......................................................................
Patch Set 6:
Looks like PS5 is a draft, so my comments didn't get published. Given the diff between 5..6 is pretty minimal, I think all my comments also apply to PS6.
--
To view, visit http://gerrit.cloudera.org:8080/12568
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic857c38076d973ad749a41fecd1b470c7881db5e
Gerrit-Change-Number: 12568
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 26 Feb 2019 04:04:02 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7645: Add a query option to set the default table file format
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12568 )
Change subject: IMPALA-7645: Add a query option to set the default table file format
......................................................................
Patch Set 8: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/12568
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic857c38076d973ad749a41fecd1b470c7881db5e
Gerrit-Change-Number: 12568
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 27 Feb 2019 03:37:09 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7645: Add a query option to set the default table file format
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12568 )
Change subject: IMPALA-7645: Add a query option to set the default table file format
......................................................................
Patch Set 6: Code-Review+1
(1 comment)
Carry Tim's +1.
http://gerrit.cloudera.org:8080/#/c/12568/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/12568/4//COMMIT_MSG@9
PS4, Line 9: DEFAULT_FILE_FORM
> Maybe DEFAULT_FILE_FORMAT? Only because people might be confused about whet
Makes sense. Done.
--
To view, visit http://gerrit.cloudera.org:8080/12568
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic857c38076d973ad749a41fecd1b470c7881db5e
Gerrit-Change-Number: 12568
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 26 Feb 2019 03:45:54 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7645: Add a query option to set the default table file format
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12568 )
Change subject: IMPALA-7645: Add a query option to set the default table file format
......................................................................
Patch Set 8: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/12568
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic857c38076d973ad749a41fecd1b470c7881db5e
Gerrit-Change-Number: 12568
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 27 Feb 2019 07:49:25 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7645: Add a query option to set the default table file format
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/12568 )
Change subject: IMPALA-7645: Add a query option to set the default table file format
......................................................................
IMPALA-7645: Add a query option to set the default table file format
This patch adds a query option "TABLE_FILE_FORMAT" to allow setting the
default table file format. Below is the list of supported
"TABLE_FILE_FORMAT" query option values:
- TEXT
- RC_FILE
- SEQUENCE_FILE
- AVRO
- PARQUET
- KUDU
- ORC
For backward compatibility, the default table file format remains as
TEXT.
Testing:
- Ran all FE tests
- Added BE test
- Added E2E tests
- Ran test_ddl.py and test_set.py
Change-Id: Ic857c38076d973ad749a41fecd1b470c7881db5e
---
M be/src/service/query-options-test.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/TableDef.java
M testdata/workloads/functional-query/queries/QueryTest/set.test
M tests/metadata/test_ddl.py
9 files changed, 95 insertions(+), 9 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/68/12568/4
--
To view, visit http://gerrit.cloudera.org:8080/12568
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic857c38076d973ad749a41fecd1b470c7881db5e
Gerrit-Change-Number: 12568
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-7645: Add a query option to set the default table file format
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12568 )
Change subject: IMPALA-7645: Add a query option to set the default table file format
......................................................................
Patch Set 4:
Would be good if someone who knew the FE well could take a look
--
To view, visit http://gerrit.cloudera.org:8080/12568
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic857c38076d973ad749a41fecd1b470c7881db5e
Gerrit-Change-Number: 12568
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 25 Feb 2019 21:19:32 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7645: Add a query option to set the default table file format
Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Paul Rogers has posted comments on this change. ( http://gerrit.cloudera.org:8080/12568 )
Change subject: IMPALA-7645: Add a query option to set the default table file format
......................................................................
Patch Set 6:
Can the file format be set in the sql statement itself? I always consider it a bit of a hack to have to set a query option before running a statement: just one more thing for the user to remember. And, to remember to set it back afterward.
--
To view, visit http://gerrit.cloudera.org:8080/12568
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic857c38076d973ad749a41fecd1b470c7881db5e
Gerrit-Change-Number: 12568
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 26 Feb 2019 03:47:01 +0000
Gerrit-HasComments: No