[Impala-ASF-CR] IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
Zach Amsden has abandoned this change. Change subject: IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT .. Abandoned Not doing at this time. -- To view, visit http://gerrit.cloudera.org:8080/6261 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: abandon Gerrit-Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633 Gerrit-PatchSet: 14 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zach Amsden Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Zach Amsden
[Impala-ASF-CR] IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
Hello Matthew Jacobs, Dan Hecht, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/6261 to look at the new patch set (#14). Change subject: IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT .. IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT Create external table can be used for two different operations, creating a new table by selecting data from another, and importing an existing unmanaged external table. Previously, for Kudu tables, it was only possible to issue CTAS for managed tables, and external tables were assumed to be imports of already existing data. This change allows the creation of new, unmanaged (external) kudu tables from SELECT statements. In the catalog executor, we detect if a table creation is new by inferring that from the presence of column definitions, not by whether it is external / managed. Front-end code is changed to allow this through the parser. This means any attempts to specify table metadata for Kudu tables are now interpreted as a table creation. Testing: Manual testing, expanded ParserTest and AnalyzeDDLTest and added test cases to query_test to validate Kudu support. Ran the folowing query: create external table test2_name primary key(id) partition by hash(id) partitions 10 stored as kudu as select * from test_name; Which successfully created and inserted 3 rows of data. Tried many other things like changing primary keys and forms of partitioning. Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633 --- M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M tests/query_test/test_kudu.py 8 files changed, 132 insertions(+), 38 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/61/6261/14 -- To view, visit http://gerrit.cloudera.org:8080/6261 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633 Gerrit-PatchSet: 14 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zach Amsden Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Zach Amsden
[Impala-ASF-CR] IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
Zach Amsden has posted comments on this change. Change subject: IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT .. Patch Set 13: rebased as this has bit-rotted, looks like an upstream toolchain package change? -- To view, visit http://gerrit.cloudera.org:8080/6261 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633 Gerrit-PatchSet: 13 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zach Amsden Gerrit-Reviewer: Alex Behm Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Zach Amsden Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
Zach Amsden has posted comments on this change.
Change subject: IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
..
Patch Set 12:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6261/12/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java:
Line 2265: AnalyzesOk("create external table t (x int primary key)
partition by hash (x) " +
> What does this statement do after your changes? Will it create a new Kudu t
It will attempt to create a new, unmanaged Kudu table with the default Impala
naming convention.
--
To view, visit http://gerrit.cloudera.org:8080/6261
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633
Gerrit-PatchSet: 12
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Dan Hecht
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Marcel Kornacker
Gerrit-Reviewer: Matthew Jacobs
Gerrit-Reviewer: Thomas Tauber-Marshall
Gerrit-Reviewer: Zach Amsden
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
Alex Behm has posted comments on this change.
Change subject: IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
..
Patch Set 12:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6261/12/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java:
Line 2265: AnalyzesOk("create external table t (x int primary key)
partition by hash (x) " +
What does this statement do after your changes? Will it create a new Kudu table
since column definitions are given?
--
To view, visit http://gerrit.cloudera.org:8080/6261
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633
Gerrit-PatchSet: 12
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden
Gerrit-Reviewer: Alex Behm
Gerrit-Reviewer: Dan Hecht
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Marcel Kornacker
Gerrit-Reviewer: Matthew Jacobs
Gerrit-Reviewer: Thomas Tauber-Marshall
Gerrit-Reviewer: Zach Amsden
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
Zach Amsden has posted comments on this change. Change subject: IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT .. Patch Set 11: (1 comment) http://gerrit.cloudera.org:8080/#/c/6261/11/shell/impala_shell.py File shell/impala_shell.py: Line 49: HISTORY_LENGTH = 1 > revert that Whoops, this was supposed to be a local only change. -- To view, visit http://gerrit.cloudera.org:8080/6261 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633 Gerrit-PatchSet: 11 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zach Amsden Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Zach Amsden Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
Hello Matthew Jacobs, Dan Hecht, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/6261 to look at the new patch set (#12). Change subject: IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT .. IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT Create external table can be used for two different operations, creating a new table by selecting data from another, and importing an existing unmanaged external table. Previously, for Kudu tables, it was only possible to issue CTAS for managed tables, and external tables were assumed to be imports of already existing data. This change allows the creation of new, unmanaged (external) kudu tables from SELECT statements. In the catalog executor, we detect if a table creation is new by inferring that from the presence of column definitions, not by whether it is external / managed. Front-end code is changed to allow this through the parser. This means any attempts to specify table metadata for Kudu tables are now interpreted as a table creation. Testing: Manual testing, expanded ParserTest and AnalyzeDDLTest and added test cases to query_test to validate Kudu support. Ran the folowing query: create external table test2_name primary key(id) partition by hash(id) partitions 10 stored as kudu as select * from test_name; Which successfully created and inserted 3 rows of data. Tried many other things like changing primary keys and forms of partitioning. Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633 --- M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M tests/query_test/test_kudu.py 7 files changed, 91 insertions(+), 32 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/61/6261/12 -- To view, visit http://gerrit.cloudera.org:8080/6261 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633 Gerrit-PatchSet: 12 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zach Amsden Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Zach Amsden
[Impala-ASF-CR] IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
Dan Hecht has posted comments on this change. Change subject: IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT .. Patch Set 11: (1 comment) http://gerrit.cloudera.org:8080/#/c/6261/11/shell/impala_shell.py File shell/impala_shell.py: Line 49: HISTORY_LENGTH = 1 revert that -- To view, visit http://gerrit.cloudera.org:8080/6261 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633 Gerrit-PatchSet: 11 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zach Amsden Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Zach Amsden Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
Dan Hecht has posted comments on this change. Change subject: IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT .. Patch Set 10: Code-Review+1 Thanks, the new commit message is much clearer. -- To view, visit http://gerrit.cloudera.org:8080/6261 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633 Gerrit-PatchSet: 10 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zach Amsden Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Zach Amsden Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
Hello Matthew Jacobs, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/6261 to look at the new patch set (#10). Change subject: IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT .. IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT Create external table can be used for two different operations, creating a new table by selecting data from another, and importing an existing unmanaged external table. Previously, for Kudu tables, it was only possible to issue CTAS for managed tables, and external tables were assumed to be imports of already existing data. This change allows the creation of new, unmanaged (external) kudu tables from SELECT statements. In the catalog executor, we detect if a table creation is new by inferring that from the presence of column definitions, not by whether it is external / managed. Front-end code is changed to allow this through the parser. This means any attempts to specify table metadata for Kudu tables are now interpreted as a table creation. Testing: Manual testing, expanded ParserTest and AnalyzeDDLTest and added test cases to query_test to validate Kudu support. Ran the folowing query: create external table test2_name primary key(id) partition by hash(id) partitions 10 stored as kudu as select * from test_name; Which successfully created and inserted 3 rows of data. Tried many other things like changing primary keys and forms of partitioning. Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633 --- M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M shell/impala_shell.py M tests/query_test/test_kudu.py 8 files changed, 92 insertions(+), 33 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/61/6261/10 -- To view, visit http://gerrit.cloudera.org:8080/6261 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633 Gerrit-PatchSet: 10 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zach Amsden Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Zach Amsden
[Impala-ASF-CR] IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
Zach Amsden has posted comments on this change. Change subject: IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT .. Patch Set 9: (1 comment) http://gerrit.cloudera.org:8080/#/c/6261/9//COMMIT_MSG Commit Message: Line 10: creating a new managed table, and importing an existing unmanaged > "Managed" is a synonym for "internal", and the opposite of "external". So, Code is right, comment is very confused. Let's try this: Create external table can be used for two different operations, creating a new table by selecting data from another, and importing an existing unmanaged external table. Previously, for Kudu tables, it was only possible to issue CTAS for managed tables, and external tables were assumed to be imports of already existing data. This change allows the creation of new, unmanaged (external) kudu tables from SELECT statements. In the catalog executor, we detect if a table creation is new by inferring that from the presence of column definitions, not by whether it is external / managed. Front-end code is changed to allow this through the parser. This means any attempts to specify table metadata for Kudu tables are now interpreted as a table creation. -- To view, visit http://gerrit.cloudera.org:8080/6261 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633 Gerrit-PatchSet: 9 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zach Amsden Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Zach Amsden Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
Dan Hecht has posted comments on this change. Change subject: IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT .. Patch Set 9: (1 comment) http://gerrit.cloudera.org:8080/#/c/6261/9//COMMIT_MSG Commit Message: Line 10: creating a new managed table, and importing an existing unmanaged "Managed" is a synonym for "internal", and the opposite of "external". So, I don't understand what you mean by "creating a new managed table" when it comes to "create external table". The semantics for external tables is that DROP TABLE won't delete the backing data. That should be true for all external tables, whether Impala had to create the table or the table was pre-existing. So, what do you mean exactly by "managed" here? -- To view, visit http://gerrit.cloudera.org:8080/6261 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633 Gerrit-PatchSet: 9 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zach Amsden Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Zach Amsden Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
Hello Matthew Jacobs, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/6261 to look at the new patch set (#8). Change subject: IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT .. IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT Create external table can be used for two different operations, creating a new managed table, and importing an existing unmanaged external table. Previously, it was only possible to create managed external tables with CTAS for Kudu. This change makes it possible to create new, unmanaged external kudu tables from SELECT statements. This is useful for importing data from one table to another persistent table in Kudu. In the catalog executor, we detect if a table creation is new by inferring that from the presence of column definitions, not by whether it is external / managed. Front-end code is changed to allow this through the parser. This means any attempts to specify table metadata for Kudu tables are interpreted as a table creation. Testing: Manual testing, expanded ParserTest and AnalyzeDDLTest and added test cases to query_test to validate Kudu support. Ran the folowing query: create external table test2_name primary key(id) partition by hash(id) partitions 10 stored as kudu as select * from test_name; Which successfully created and inserted 3 rows of data. Tried many other things like changing primary keys and forms of partitioning. Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633 --- M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M shell/impala_shell.py M tests/query_test/test_kudu.py 8 files changed, 92 insertions(+), 33 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/61/6261/8 -- To view, visit http://gerrit.cloudera.org:8080/6261 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633 Gerrit-PatchSet: 8 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zach Amsden Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Zach Amsden
[Impala-ASF-CR] IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
Zach Amsden has posted comments on this change.
Change subject: IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
..
Patch Set 7:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/6261/7//COMMIT_MSG
Commit Message:
Line 10: new by inferring that from the presence of column definitions, not
> what does 'if a table creation is new' mean?
New rather that existing table. Terminology here is confusing, as managed !=
external, yet in many places of code and documentation, that is implied. Will
rephrase.
http://gerrit.cloudera.org:8080/#/c/6261/7/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java:
Line 2264: "tblproperties('kudu.table_name'='t')", "Table partitioning
must be " +
> why is that? wouldn't the kudu table/metadata be able to provide that?
Specifying any part of the metadata (x int) now implies a table creation, as we
use the presence of column definitions to infer that this is a table creation.
Since the assumption is that the table is being created, we need to know the
partitioning scheme to create it. I tried to make the error message suggest
this. I'll add a positive test case to illustrate the required syntax.
http://gerrit.cloudera.org:8080/#/c/6261/6/tests/query_test/test_kudu.py
File tests/query_test/test_kudu.py:
PS6, Line 360: kudu_tbl_name =
KuduTestSuite.to_kudu_table_name(unique_database, "foo2")
: assert kudu_client.table_exists(kudu_tbl_name)
: cursor.execute("SELECT * FROM %s.foo2" % (unique_database))
: assert len(cursor.fetchall()) == 1
: cursor.execute("DROP TABLE %s.foo2" % (unique_database))
: assert kudu_client.table_exists(kudu_tbl_name)
> foo2 in kudu won't be deleted when you drop the database in impala though,
Good point.
--
To view, visit http://gerrit.cloudera.org:8080/6261
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden
Gerrit-Reviewer: Dan Hecht
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Marcel Kornacker
Gerrit-Reviewer: Matthew Jacobs
Gerrit-Reviewer: Thomas Tauber-Marshall
Gerrit-Reviewer: Zach Amsden
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
Marcel Kornacker has posted comments on this change.
Change subject: IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
..
Patch Set 7:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/6261/7//COMMIT_MSG
Commit Message:
Line 10: new by inferring that from the presence of column definitions, not
what does 'if a table creation is new' mean?
http://gerrit.cloudera.org:8080/#/c/6261/7/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java:
Line 2264: "tblproperties('kudu.table_name'='t')", "Table partitioning
must be " +
why is that? wouldn't the kudu table/metadata be able to provide that?
--
To view, visit http://gerrit.cloudera.org:8080/6261
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633
Gerrit-PatchSet: 7
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden
Gerrit-Reviewer: Dan Hecht
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Marcel Kornacker
Gerrit-Reviewer: Matthew Jacobs
Gerrit-Reviewer: Thomas Tauber-Marshall
Gerrit-Reviewer: Zach Amsden
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
Marcel Kornacker has posted comments on this change. Change subject: IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT .. Patch Set 7: i'm going to have a look -- To view, visit http://gerrit.cloudera.org:8080/6261 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633 Gerrit-PatchSet: 7 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zach Amsden Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Marcel Kornacker Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Zach Amsden Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
Matthew Jacobs has posted comments on this change. Change subject: IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT .. Patch Set 7: Code-Review+1 (1 comment) http://gerrit.cloudera.org:8080/#/c/6261/7/fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java File fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java: PS7, Line 65: if 'msTbl' represents an external table update this comment -- To view, visit http://gerrit.cloudera.org:8080/6261 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633 Gerrit-PatchSet: 7 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zach Amsden Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Zach Amsden Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
Matthew Jacobs has posted comments on this change.
Change subject: IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
..
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/6261/6/tests/query_test/test_kudu.py
File tests/query_test/test_kudu.py:
PS6, Line 360: kudu_tbl_name =
KuduTestSuite.to_kudu_table_name(unique_database, "foo2")
: assert kudu_client.table_exists(kudu_tbl_name)
: cursor.execute("SELECT * FROM %s.foo2" % (unique_database))
: assert len(cursor.fetchall()) == 1
: cursor.execute("DROP TABLE %s.foo2" % (unique_database))
: assert kudu_client.table_exists(kudu_tbl_name)
> Unless I'm mistaken, the entire unique_database gets dropped at the end of
foo2 in kudu won't be deleted when you drop the database in impala though,
that's the point of the external table. if you take a look at your kudu I bet
you'll see a bunch of foo2 tables around (I think you can look at the master's
web ui)
--
To view, visit http://gerrit.cloudera.org:8080/6261
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden
Gerrit-Reviewer: Dan Hecht
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Matthew Jacobs
Gerrit-Reviewer: Thomas Tauber-Marshall
Gerrit-Reviewer: Zach Amsden
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
Zach Amsden has uploaded a new patch set (#7). Change subject: IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT .. IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT In the catalog executor, we can detect if a table creation is new by inferring that from the presence of column definitions, not by whether it is external / managed. Front-end code is changed to allow this through the parser. Testing: Manual testing, expanded ParserTest and AnalyzeDDLTest and added test cases to query_test to validate Kudu support. Ran the folowing query: create external table test10_name primary key(id) partition by hash(id) partitions 10 stored as kudu as select * from test_name; Which successfully created and inserted 3 rows of data. Tried a few other things like changing primary key and partitioning. Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633 --- M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M tests/query_test/test_kudu.py 7 files changed, 82 insertions(+), 30 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/61/6261/7 -- To view, visit http://gerrit.cloudera.org:8080/6261 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633 Gerrit-PatchSet: 7 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zach Amsden Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Zach Amsden
[Impala-ASF-CR] IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
Zach Amsden has posted comments on this change.
Change subject: IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
..
Patch Set 6:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/6261/6/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
File fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java:
PS6, Line 201: Ingested
> how about analyzeExistingKuduTableParams ?
Done
http://gerrit.cloudera.org:8080/#/c/6261/6/fe/src/test/java/org/apache/impala/analysis/ParserTest.java
File fe/src/test/java/org/apache/impala/analysis/ParserTest.java:
Line 2793:
> nit extra line
Done
http://gerrit.cloudera.org:8080/#/c/6261/6/tests/query_test/test_kudu.py
File tests/query_test/test_kudu.py:
PS6, Line 360: kudu_tbl_name =
KuduTestSuite.to_kudu_table_name(unique_database, "foo2")
: assert kudu_client.table_exists(kudu_tbl_name)
: cursor.execute("SELECT * FROM %s.foo2" % (unique_database))
: assert len(cursor.fetchall()) == 1
: cursor.execute("DROP TABLE %s.foo2" % (unique_database))
: assert kudu_client.table_exists(kudu_tbl_name)
> I think you wanna wrap this in a try/finally and make sure to drop the kudu
Unless I'm mistaken, the entire unique_database gets dropped at the end of the
test. The only reason I'm dropping the tables at all here is to test the
external vs. managed table behavior. See for example test_kudu_col_removed,
test_kudu_rename_table which don't bother to clean up.
--
To view, visit http://gerrit.cloudera.org:8080/6261
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden
Gerrit-Reviewer: Dan Hecht
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Matthew Jacobs
Gerrit-Reviewer: Thomas Tauber-Marshall
Gerrit-Reviewer: Zach Amsden
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
Matthew Jacobs has posted comments on this change.
Change subject: IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
..
Patch Set 6:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/6261/6/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
File fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java:
PS6, Line 201: Ingested
how about analyzeExistingKuduTableParams ?
http://gerrit.cloudera.org:8080/#/c/6261/6/fe/src/test/java/org/apache/impala/analysis/ParserTest.java
File fe/src/test/java/org/apache/impala/analysis/ParserTest.java:
Line 2793:
nit extra line
http://gerrit.cloudera.org:8080/#/c/6261/6/tests/query_test/test_kudu.py
File tests/query_test/test_kudu.py:
PS6, Line 360: kudu_tbl_name =
KuduTestSuite.to_kudu_table_name(unique_database, "foo2")
: assert kudu_client.table_exists(kudu_tbl_name)
: cursor.execute("SELECT * FROM %s.foo2" % (unique_database))
: assert len(cursor.fetchall()) == 1
: cursor.execute("DROP TABLE %s.foo2" % (unique_database))
: assert kudu_client.table_exists(kudu_tbl_name)
I think you wanna wrap this in a try/finally and make sure to drop the kudu
table.
--
To view, visit http://gerrit.cloudera.org:8080/6261
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden
Gerrit-Reviewer: Dan Hecht
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Matthew Jacobs
Gerrit-Reviewer: Thomas Tauber-Marshall
Gerrit-Reviewer: Zach Amsden
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
Zach Amsden has uploaded a new patch set (#5). Change subject: IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT .. IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT In the catalog executor, we can detect if a table creation is new by inferring that from the presence of column definitions, not by whether it is external / managed. Front-end code is changed to allow this through the parser. Testing: Manual testing, expanded ParserTest and AnalyzeDDLTest and added test cases to query_test to validate Kudu support. Ran the folowing query: create external table test10_name primary key(id) partition by hash(id) partitions 10 stored as kudu as select * from test_name; Which successfully created and inserted 3 rows of data. Tried a few other things like changing primary key and partitioning. Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633 --- M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M tests/query_test/test_kudu.py 7 files changed, 83 insertions(+), 30 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/61/6261/5 -- To view, visit http://gerrit.cloudera.org:8080/6261 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zach Amsden Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Zach Amsden
[Impala-ASF-CR] IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
Zach Amsden has posted comments on this change. Change subject: IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT .. Patch Set 4: (2 comments) Thanks for the review! http://gerrit.cloudera.org:8080/#/c/6261/4//COMMIT_MSG Commit Message: PS4, Line 9: No reason not to allow this. > Not necessary. Will fix http://gerrit.cloudera.org:8080/#/c/6261/4/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java File fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java: Line 240:* Analyzes and checks parameters specified for ingested external Kudu tables. > Since ingested isn't a term we use anywhere else (AFAIK), you should explai Done -- To view, visit http://gerrit.cloudera.org:8080/6261 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zach Amsden Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Zach Amsden Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
Thomas Tauber-Marshall has posted comments on this change. Change subject: IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT .. Patch Set 4: (2 comments) http://gerrit.cloudera.org:8080/#/c/6261/4//COMMIT_MSG Commit Message: PS4, Line 9: No reason not to allow this. Not necessary. Also, I find this whole paragraph confusing - for example what thrift protocol? Why would it need to be changed? (not that I'm saying you should explain this, it may be an unnecessary detail). Obviously, you should assume that anyone reading this has no context for the work that you've been doing up to this point. http://gerrit.cloudera.org:8080/#/c/6261/4/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java File fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java: Line 240:* Analyzes and checks parameters specified for ingested external Kudu tables. Since ingested isn't a term we use anywhere else (AFAIK), you should explain what you mean by it, something like: "Analyzes and checks parameters specified for external tables that already exist in Kudu." -- To view, visit http://gerrit.cloudera.org:8080/6261 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zach Amsden Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Zach Amsden Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
Zach Amsden has uploaded a new patch set (#4). Change subject: IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT .. IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT No reason not to allow this. Rather than change the thrift protocol, in the catalog executor, we detect if a table creation is new by inferring that from the presence of column definitions, not by whether it is external / managed. Front-end code is changed to allow this through the parser. Testing: Manual testing, expanded ParserTest and AnalyzeDDLTest and added test cases to query_test to validate Kudu support. Ran the folowing query: create external table test10_name primary key(id) partition by hash(id) partitions 10 stored as kudu as select * from test_name; Which successfully created and inserted 3 rows of data. Tried a few other things like changing primary key and partitioning. Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633 --- M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M tests/query_test/test_kudu.py 7 files changed, 83 insertions(+), 30 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/61/6261/4 -- To view, visit http://gerrit.cloudera.org:8080/6261 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zach Amsden Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Zach Amsden
[Impala-ASF-CR] IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
Zach Amsden has uploaded a new patch set (#3). Change subject: IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT .. IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT No reason not to allow this. Rather than change the thrift protocol, in the catalog executor, we detect if a table creation is new by inferring that from the presence of column definitions, not by whether it is external / managed. Front-end code is changed to allow this through the parser. Testing: Manual testing. Currently I can't run the DDL test as I am reloading test data. Ran the folowing query: create external table test10_name primary key(id) partition by hash(id) partitions 10 stored as kudu as select * from test_name; Which successfully created and inserted 3 rows of data. Tried a few other things like changing primary key and partitioning, which all seemed to work. Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633 --- M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java 5 files changed, 28 insertions(+), 16 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/61/6261/3 -- To view, visit http://gerrit.cloudera.org:8080/6261 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zach Amsden Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Zach Amsden
[Impala-ASF-CR] IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
Zach Amsden has posted comments on this change.
Change subject: IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
..
Patch Set 1:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/6261/1/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
File fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java:
PS1, Line 210: !createAsSelect
> how about instead of this, checking if there were columns specified or not?
Done
PS1, Line 211: analyzeExternalKuduTableParams(analyzer);
: } else {
: analyzeManagedKuduTableParams(analyzer);
> It might be easier to understand if we rebrand these functions now, e.g. an
yeah, doing that.
http://gerrit.cloudera.org:8080/#/c/6261/2/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
File fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java:
PS2, Line 57: // Is this a CREATE TABLE ... AS SELECT statment
: private boolean createAsSelect_ = false;
> This just doesn't feel right to me, especially given that this is only used
The "best" thing to do is capture state about whether the Table parameters have
defined any columns. We already have that state, and use it in the backend, so
I think I will just kill this.
http://gerrit.cloudera.org:8080/#/c/6261/2/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
PS2, Line 1649:
> why remove this?
misread the precondition as isExternalTable, I'll add it back
--
To view, visit http://gerrit.cloudera.org:8080/6261
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden
Gerrit-Reviewer: Dan Hecht
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Matthew Jacobs
Gerrit-Reviewer: Zach Amsden
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
Matthew Jacobs has posted comments on this change.
Change subject: IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
..
Patch Set 1:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/6261/1/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
File fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java:
PS1, Line 210: !createAsSelect
how about instead of this, checking if there were columns specified or not?
PS1, Line 211: analyzeExternalKuduTableParams(analyzer);
: } else {
: analyzeManagedKuduTableParams(analyzer);
It might be easier to understand if we rebrand these functions now, e.g.
analyzeExistingKuduTableParams and analyzeNewKuduTableParams.
http://gerrit.cloudera.org:8080/#/c/6261/2/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
File fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java:
PS2, Line 57: // Is this a CREATE TABLE ... AS SELECT statment
: private boolean createAsSelect_ = false;
This just doesn't feel right to me, especially given that this is only used on
the Kudu path.
http://gerrit.cloudera.org:8080/#/c/6261/2/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
PS2, Line 1649:
why remove this?
--
To view, visit http://gerrit.cloudera.org:8080/6261
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden
Gerrit-Reviewer: Dan Hecht
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Matthew Jacobs
Gerrit-Reviewer: Zach Amsden
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
Zach Amsden has uploaded a new patch set (#2). Change subject: IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT .. IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT No reason not to allow this. Rather than change the thrift protocol, in the catalog executor, we detect if a table creation is new by inferring that from the presence of column definitions, not by whether it is external / managed. Front-end code is changed to allow this through the parser. Testing: Manual testing. I could appreciate some pointers as to where to find the additional unit tests to modify. ParserTest coverage for these cases is pretty marginal, that at least should be threshed out a bit more but I am not sure what else. Ran the folowing query: create external table test10_name primary key(id) partition by hash(id) partitions 10 stored as kudu as select * from test_name; Which successfully created and inserted 3 rows of data. Tried a few other things like changing primary key and partitioning, which all seemed to work. Update: less confusing name Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633 --- M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java 5 files changed, 20 insertions(+), 14 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/61/6261/2 -- To view, visit http://gerrit.cloudera.org:8080/6261 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zach Amsden Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Dimitris Tsirogiannis Gerrit-Reviewer: Matthew Jacobs Gerrit-Reviewer: Zach Amsden
[Impala-ASF-CR] IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
Zach Amsden has posted comments on this change.
Change subject: IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
..
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/6261/1/fe/src/main/cup/sql-parser.cup
File fe/src/main/cup/sql-parser.cup:
PS1, Line 1033: CreateTableStmt.CreateTableAsSelectStmt
> maybe I'll see why this is necessary later, but this seems odd since it loo
The name is confusing. It's just a static factory method for creating a
CreatTableStmt that is going to be used as a sub-part of a
CreateTableAsSelectStmt (and it needs to know that).
http://gerrit.cloudera.org:8080/#/c/6261/1/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
File fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java:
PS1, Line 68: public static CreateTableStmt CreateTableAsSelectStmt(TableDef
tableDef) {
: CreateTableStmt stmt = new CreateTableStmt(tableDef);
: stmt.createAsSelect_ = true;
: return stmt;
: }
> yeah, per my comment in sql-parser this seems odd to have this parameter on
I didn't want to override the constructor for CreateTableStmt with a boolean
parameter 'isCreateAsSelect' - in 2 years nobody is going to know why some
boolean true is passed or what it does and it isn't clear from the calling
context. So instead I created a static factory method for CreateTableStmt that
tells you exactly what it is doing.
Perhaps a more terse name would be better and it does look confusing that it is
named the same as another statement.
I can't just set isExternal = true because that can also be true without the
context of CTAS. We need to know whether this table is to be created from a
select, or have column data ingested from an external table. The fact that it
is an external table is now orthogonal to that.
--
To view, visit http://gerrit.cloudera.org:8080/6261
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden
Gerrit-Reviewer: Dan Hecht
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Matthew Jacobs
Gerrit-Reviewer: Zach Amsden
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
Matthew Jacobs has posted comments on this change.
Change subject: IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
..
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/6261/1/fe/src/main/cup/sql-parser.cup
File fe/src/main/cup/sql-parser.cup:
PS1, Line 1033: CreateTableStmt.CreateTableAsSelectStmt
maybe I'll see why this is necessary later, but this seems odd since it looks
like it would create a CreateTableAsSelectStmt, only to be passed as an arg to
CreateTableAsSelectStmt.
http://gerrit.cloudera.org:8080/#/c/6261/1/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
File fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java:
PS1, Line 68: public static CreateTableStmt CreateTableAsSelectStmt(TableDef
tableDef) {
: CreateTableStmt stmt = new CreateTableStmt(tableDef);
: stmt.createAsSelect_ = true;
: return stmt;
: }
yeah, per my comment in sql-parser this seems odd to have this parameter on
CreateTableStmt while the CreateTableAsSelectStmt consumes this.
why can't you just set isExternal = true in the TableDef? Maybe I'm missing
something?
--
To view, visit http://gerrit.cloudera.org:8080/6261
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Zach Amsden
Gerrit-Reviewer: Dan Hecht
Gerrit-Reviewer: Dimitris Tsirogiannis
Gerrit-Reviewer: Matthew Jacobs
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT
Zach Amsden has uploaded a new change for review. http://gerrit.cloudera.org:8080/6261 Change subject: IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT .. IMPALA-4318: Kudu support for CREATE EXTERNAL TABLE AS SELECT No reason not to allow this. Rather than change the thrift protocol, in the catalog executor, we detect if a table creation is new by inferring that from the presence of column definitions, not by whether it is external / managed. Front-end code is changed to allow this through the parser. Testing: Manual testing. I could appreciate some pointers as to where to find the additional unit tests to modify. ParserTest coverage for these cases is pretty marginal, that at least should be threshed out a bit more but I am not sure what else. Ran the folowing query: create external table test10_name primary key(id) partition by hash(id) partitions 10 stored as kudu as select * from test_name; Which successfully created and inserted 3 rows of data. Tried a few other things like changing primary key and partitioning, which all seemed to work. Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633 --- M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java 5 files changed, 21 insertions(+), 14 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/61/6261/1 -- To view, visit http://gerrit.cloudera.org:8080/6261 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I9aa82809a6c0c5e6386827314b7e5b520c1a6633 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Zach Amsden
