You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org> on 2020/04/10 12:13:14 UTC

[Impala-ASF-CR] Put transactional tables into 'managed' directory

Zoltan Borok-Nagy has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15708


Change subject: Put transactional tables into 'managed' directory
......................................................................

Put transactional tables into 'managed' directory

HIVE-22794 disallows ACID tables outside of the 'managed' warehouse
directory. This change updates data loading to make it conform to
the new rules.

The following tests had to be modified to use the new paths:
* AnalyzeDDLTest.TestCreateTableLikeFileOrc()
* create-table-like-file-orc.test

Change-Id: Id3b65f56bf7f225b1d29aa397f987fdd7eb7176c
---
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M testdata/bin/generate-schema-statements.py
M testdata/datasets/functional/functional_schema_template.sql
M testdata/workloads/functional-query/queries/QueryTest/create-table-like-file-orc.test
4 files changed, 28 insertions(+), 40 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/08/15708/1
-- 
To view, visit http://gerrit.cloudera.org:8080/15708
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id3b65f56bf7f225b1d29aa397f987fdd7eb7176c
Gerrit-Change-Number: 15708
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] Put transactional tables into 'managed' directory

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15708 )

Change subject: Put transactional tables into 'managed' directory
......................................................................


Patch Set 1: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/15708
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id3b65f56bf7f225b1d29aa397f987fdd7eb7176c
Gerrit-Change-Number: 15708
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 10 Apr 2020 16:44:08 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] Put transactional tables into 'managed' directory

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/15708 )

Change subject: Put transactional tables into 'managed' directory
......................................................................


Patch Set 1: Code-Review+2

(1 comment)

This makes sense to me.

http://gerrit.cloudera.org:8080/#/c/15708/1/testdata/datasets/functional/functional_schema_template.sql
File testdata/datasets/functional/functional_schema_template.sql:

http://gerrit.cloudera.org:8080/#/c/15708/1/testdata/datasets/functional/functional_schema_template.sql@335
PS1, Line 335: ---- TABLE_PROPERTIES
             : transactional=false
Let me double check my understanding: this is needed because this is an external table, so it goes to the external warehouse.



-- 
To view, visit http://gerrit.cloudera.org:8080/15708
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id3b65f56bf7f225b1d29aa397f987fdd7eb7176c
Gerrit-Change-Number: 15708
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Fri, 10 Apr 2020 16:54:28 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] Put transactional tables into 'managed' directory

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15708 )

Change subject: Put transactional tables into 'managed' directory
......................................................................


Patch Set 2:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5639/ DRY_RUN=false


-- 
To view, visit http://gerrit.cloudera.org:8080/15708
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id3b65f56bf7f225b1d29aa397f987fdd7eb7176c
Gerrit-Change-Number: 15708
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 10 Apr 2020 20:08:34 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] Put transactional tables into 'managed' directory

Posted by "Alice Fan (Code Review)" <ge...@cloudera.org>.
Alice Fan has posted comments on this change. ( http://gerrit.cloudera.org:8080/15708 )

Change subject: Put transactional tables into 'managed' directory
......................................................................


Patch Set 1: Code-Review+1

(1 comment)

Thanks Zoltan for the quick fix. Change looks good to me. Just have one small question.

http://gerrit.cloudera.org:8080/#/c/15708/1/testdata/datasets/functional/functional_schema_template.sql
File testdata/datasets/functional/functional_schema_template.sql:

http://gerrit.cloudera.org:8080/#/c/15708/1/testdata/datasets/functional/functional_schema_template.sql@289
PS1, Line 289: CREATE EXTERNAL TABLE IF NOT EXISTS {db_name}{db_suffix}.{table_name} (
Just curious, if file format is orc and transnational does not set here. So this table is actually created as a managed table?



-- 
To view, visit http://gerrit.cloudera.org:8080/15708
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id3b65f56bf7f225b1d29aa397f987fdd7eb7176c
Gerrit-Change-Number: 15708
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Comment-Date: Fri, 10 Apr 2020 18:17:00 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] Put transactional tables into 'managed' directory

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15708 )

Change subject: Put transactional tables into 'managed' directory
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/5782/ : 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/15708
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id3b65f56bf7f225b1d29aa397f987fdd7eb7176c
Gerrit-Change-Number: 15708
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 10 Apr 2020 12:54:31 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] Put transactional tables into 'managed' directory

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15708 )

Change subject: Put transactional tables into 'managed' directory
......................................................................


Patch Set 2: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/15708
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id3b65f56bf7f225b1d29aa397f987fdd7eb7176c
Gerrit-Change-Number: 15708
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Sat, 11 Apr 2020 00:36:55 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] Put transactional tables into 'managed' directory

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15708 )

Change subject: Put transactional tables into 'managed' directory
......................................................................


Patch Set 1:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5637/ DRY_RUN=true


-- 
To view, visit http://gerrit.cloudera.org:8080/15708
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id3b65f56bf7f225b1d29aa397f987fdd7eb7176c
Gerrit-Change-Number: 15708
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 10 Apr 2020 12:14:31 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] Put transactional tables into 'managed' directory

Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/15708 )

Change subject: Put transactional tables into 'managed' directory
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/15708/1/testdata/datasets/functional/functional_schema_template.sql
File testdata/datasets/functional/functional_schema_template.sql:

http://gerrit.cloudera.org:8080/#/c/15708/1/testdata/datasets/functional/functional_schema_template.sql@289
PS1, Line 289: CREATE EXTERNAL TABLE IF NOT EXISTS {db_name}{db_suffix}.{table_name} (
> Just curious, if file format is orc and transnational does not set here. So
No, because we have this CREATE section we'll just use this CREATE TABLE statement to create the table. And since it doesn't set the table as transactional this will remain a plain old non-transactional table.

Currently we only promote tables to transactional tables that don't have custom CREATE sections.

At L335 I added a TABLE_PROPERTIES section to make the non-transactinal property explicit.


http://gerrit.cloudera.org:8080/#/c/15708/1/testdata/datasets/functional/functional_schema_template.sql@335
PS1, Line 335: ---- TABLE_PROPERTIES
             : transactional=false
> Let me double check my understanding: this is needed because this is an ext
Exactly. Without it we'd try to make it transactional in generate-schema-statements.py and set hdfs_location to point into the managed directory.

However, since it has a specific CREATE statement that doesn't configure it as transactional, we'd end up with an external non-transactional table that is located in the managed dir. And Hive already prohibit external tables inside the managed dir.

So these tables weren't transactional earlier either (due to the CREATE section), we are just making it explicit now.



-- 
To view, visit http://gerrit.cloudera.org:8080/15708
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id3b65f56bf7f225b1d29aa397f987fdd7eb7176c
Gerrit-Change-Number: 15708
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 10 Apr 2020 20:08:08 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] Put transactional tables into 'managed' directory

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15708 )

Change subject: Put transactional tables into 'managed' directory
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15708/1/testdata/bin/generate-schema-statements.py
File testdata/bin/generate-schema-statements.py:

http://gerrit.cloudera.org:8080/#/c/15708/1/testdata/bin/generate-schema-statements.py@667
PS1, Line 667: '
flake8: E129 visually indented line with same indent as next logical line



-- 
To view, visit http://gerrit.cloudera.org:8080/15708
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id3b65f56bf7f225b1d29aa397f987fdd7eb7176c
Gerrit-Change-Number: 15708
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 10 Apr 2020 12:13:59 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] Put transactional tables into 'managed' directory

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/15708 )

Change subject: Put transactional tables into 'managed' directory
......................................................................

Put transactional tables into 'managed' directory

HIVE-22794 disallows ACID tables outside of the 'managed' warehouse
directory. This change updates data loading to make it conform to
the new rules.

The following tests had to be modified to use the new paths:
* AnalyzeDDLTest.TestCreateTableLikeFileOrc()
* create-table-like-file-orc.test

Change-Id: Id3b65f56bf7f225b1d29aa397f987fdd7eb7176c
Reviewed-on: http://gerrit.cloudera.org:8080/15708
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M testdata/bin/generate-schema-statements.py
M testdata/datasets/functional/functional_schema_template.sql
M testdata/workloads/functional-query/queries/QueryTest/create-table-like-file-orc.test
4 files changed, 28 insertions(+), 40 deletions(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/15708
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Id3b65f56bf7f225b1d29aa397f987fdd7eb7176c
Gerrit-Change-Number: 15708
Gerrit-PatchSet: 3
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>

[Impala-ASF-CR] Put transactional tables into 'managed' directory

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15708 )

Change subject: Put transactional tables into 'managed' directory
......................................................................


Patch Set 2: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/15708
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id3b65f56bf7f225b1d29aa397f987fdd7eb7176c
Gerrit-Change-Number: 15708
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Alice Fan <fa...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 10 Apr 2020 20:08:33 +0000
Gerrit-HasComments: No