You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@impala.apache.org by "Anurag Mantripragada (Code Review)" <ge...@cloudera.org> on 2019/10/19 00:39:30 UTC

[Impala-ASF-CR] IMPALA-9070: Include table location in lineage for 'CREATE EXTERNAL TABLE' DDL.

Anurag Mantripragada has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14515

Change subject: IMPALA-9070: Include table location in lineage for 'CREATE EXTERNAL TABLE' DDL.
......................................................................

IMPALA-9070: Include table location in lineage for 'CREATE EXTERNAL
TABLE' DDL.

Atlas needs table location to establish lineage between a newly
created external table and its table location.

The table location information is not available until the createTable
catalog op succeeds. After this change, location information is sent
to the backend in the TDDLExecResponse message which adds it to the
lineage graph. This information is sent only for create external
table queries.

Testing:
Added a test to verify the tableLocation field is populated for a
create external table query lineage. Also, modified the
lineage.test file to include location information for all lineages.

Change-Id: If02b0cc16d52c1956298171628f5737cab62ce9f
---
M be/src/service/client-request-state.cc
M be/src/util/lineage-util.h
M common/thrift/CatalogService.thrift
M common/thrift/LineageGraph.thrift
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M testdata/workloads/functional-query/queries/QueryTest/lineage.test
M tests/custom_cluster/test_lineage.py
7 files changed, 920 insertions(+), 449 deletions(-)

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: If02b0cc16d52c1956298171628f5737cab62ce9f
Gerrit-Change-Number: 14515
Gerrit-PatchSet: 1
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>

[Impala-ASF-CR] IMPALA-9070: Include table location in lineage for 'CREATE EXTERNAL TABLE' DDL.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14515 )

Change subject: IMPALA-9070: Include table location in lineage for 'CREATE EXTERNAL TABLE' DDL.
......................................................................


Patch Set 1:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If02b0cc16d52c1956298171628f5737cab62ce9f
Gerrit-Change-Number: 14515
Gerrit-PatchSet: 1
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: radford nguyen <ra...@gmail.com>
Gerrit-Comment-Date: Sat, 19 Oct 2019 01:16:27 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9070: Include table location in lineage for 'CREATE EXTERNAL TABLE' DDL.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14515 )

Change subject: IMPALA-9070: Include table location in lineage for 'CREATE EXTERNAL TABLE' DDL.
......................................................................


Patch Set 2:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If02b0cc16d52c1956298171628f5737cab62ce9f
Gerrit-Change-Number: 14515
Gerrit-PatchSet: 2
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: radford nguyen <ra...@gmail.com>
Gerrit-Comment-Date: Mon, 21 Oct 2019 01:39:40 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9070: Include table location in lineage for 'CREATE EXTERNAL TABLE' DDL.

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.

Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14515 )

Change subject: IMPALA-9070: Include table location in lineage for 'CREATE EXTERNAL TABLE' DDL.
......................................................................


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If02b0cc16d52c1956298171628f5737cab62ce9f
Gerrit-Change-Number: 14515
Gerrit-PatchSet: 2
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: radford nguyen <ra...@gmail.com>
Gerrit-Comment-Date: Sun, 20 Oct 2019 14:59:30 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9070: Include table location in lineage for 'CREATE EXTERNAL TABLE' DDL.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14515 )

Change subject: IMPALA-9070: Include table location in lineage for 'CREATE EXTERNAL TABLE' DDL.
......................................................................


Patch Set 2: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/5115/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If02b0cc16d52c1956298171628f5737cab62ce9f
Gerrit-Change-Number: 14515
Gerrit-PatchSet: 2
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: radford nguyen <ra...@gmail.com>
Gerrit-Comment-Date: Sun, 20 Oct 2019 19:21:31 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9070: Include table location in lineage for 'CREATE EXTERNAL TABLE' DDL.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14515 )

Change subject: IMPALA-9070: Include table location in lineage for 'CREATE EXTERNAL TABLE' DDL.
......................................................................


Patch Set 2:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If02b0cc16d52c1956298171628f5737cab62ce9f
Gerrit-Change-Number: 14515
Gerrit-PatchSet: 2
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: radford nguyen <ra...@gmail.com>
Gerrit-Comment-Date: Sun, 20 Oct 2019 14:59:45 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9070: Include table location in lineage for 'CREATE EXTERNAL TABLE' DDL.

Posted by "Anurag Mantripragada (Code Review)" <ge...@cloudera.org>.

Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14515 )

Change subject: IMPALA-9070: Include table location in lineage for 'CREATE EXTERNAL TABLE' DDL.
......................................................................


Patch Set 2:

(1 comment)

You are right. Having "" can be confusing. Modified the writer to write only if location information is available.

http://gerrit.cloudera.org:8080/#/c/14515/1/be/src/util/lineage-util.h
File be/src/util/lineage-util.h:

http://gerrit.cloudera.org:8080/#/c/14515/1/be/src/util/lineage-util.h@119
PS1, Line 119:       if (lineage.__isset.table_location) {
> I think this writes it out even if it wasn't set, i.e. for external tables.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If02b0cc16d52c1956298171628f5737cab62ce9f
Gerrit-Change-Number: 14515
Gerrit-PatchSet: 2
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: radford nguyen <ra...@gmail.com>
Gerrit-Comment-Date: Sat, 19 Oct 2019 19:11:31 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9070: Include table location in lineage for 'CREATE EXTERNAL TABLE' DDL.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14515 )

Change subject: IMPALA-9070: Include table location in lineage for 'CREATE EXTERNAL TABLE' DDL.
......................................................................


Patch Set 2:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If02b0cc16d52c1956298171628f5737cab62ce9f
Gerrit-Change-Number: 14515
Gerrit-PatchSet: 2
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: radford nguyen <ra...@gmail.com>
Gerrit-Comment-Date: Sat, 19 Oct 2019 19:54:28 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9070: Include table location in lineage for 'CREATE EXTERNAL TABLE' DDL.

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.

Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14515 )

Change subject: IMPALA-9070: Include table location in lineage for 'CREATE EXTERNAL TABLE' DDL.
......................................................................


Patch Set 1:

(1 comment)

Had one question about whether "" is the right value for non-external tables.

http://gerrit.cloudera.org:8080/#/c/14515/1/be/src/util/lineage-util.h
File be/src/util/lineage-util.h:

http://gerrit.cloudera.org:8080/#/c/14515/1/be/src/util/lineage-util.h@119
PS1, Line 119:       writer.String("tableLocation");
I think this writes it out even if it wasn't set, i.e. for external tables. It seems like we should omit the key/value or set it to null (not sure what atlas expects). I.e.

 if (lineage.__isset.table_location) {
  ...
  }



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If02b0cc16d52c1956298171628f5737cab62ce9f
Gerrit-Change-Number: 14515
Gerrit-PatchSet: 1
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: radford nguyen <ra...@gmail.com>
Gerrit-Comment-Date: Sat, 19 Oct 2019 05:36:58 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9070: Include table location in lineage for 'CREATE EXTERNAL TABLE' DDL.

Posted by "Anurag Mantripragada (Code Review)" <ge...@cloudera.org>.

Anurag Mantripragada has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/14515 )

Change subject: IMPALA-9070: Include table location in lineage for 'CREATE EXTERNAL TABLE' DDL.
......................................................................

IMPALA-9070: Include table location in lineage for 'CREATE EXTERNAL
TABLE' DDL.

Atlas needs table location to establish lineage between a newly
created external table and its table location.

Testing:
Added a test to verify the tableLocation field is populated for a
create external table query lineage. Also, modified the
lineage.test file to include location information for all lineages.

git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/15/14515/2
--
To view, visit http://gerrit.cloudera.org:8080/14515
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If02b0cc16d52c1956298171628f5737cab62ce9f
Gerrit-Change-Number: 14515
Gerrit-PatchSet: 2
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: radford nguyen <ra...@gmail.com>

[Impala-ASF-CR] IMPALA-9070: Include table location in lineage for 'CREATE EXTERNAL TABLE' DDL.

Posted by "Anurag Mantripragada (Code Review)" <ge...@cloudera.org>.

Anurag Mantripragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14515 )

Change subject: IMPALA-9070: Include table location in lineage for 'CREATE EXTERNAL TABLE' DDL.
......................................................................


Patch Set 2:

The precommit job failed with an unrelated test failure. Filed https://issues.apache.org/jira/browse/IMPALA-9073 for it and re-ran the pre-commit job. Now it succeeded. Tim, could you please merge the patch now?


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If02b0cc16d52c1956298171628f5737cab62ce9f
Gerrit-Change-Number: 14515
Gerrit-PatchSet: 2
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: radford nguyen <ra...@gmail.com>
Gerrit-Comment-Date: Mon, 21 Oct 2019 15:17:38 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9070: Include table location in lineage for 'CREATE EXTERNAL TABLE' DDL.

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.

Tim Armstrong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14515 )

Change subject: IMPALA-9070: Include table location in lineage for 'CREATE EXTERNAL TABLE' DDL.
......................................................................

IMPALA-9070: Include table location in lineage for 'CREATE EXTERNAL
TABLE' DDL.

Atlas needs table location to establish lineage between a newly
created external table and its table location.

The table location information is not available until the createTable
catalog op succeeds. After this change, location information is sent
to the backend in the TDDLExecResponse message which adds it to the
lineage graph. This information is sent only for create external
table queries.

Testing:
Added a test to verify the tableLocation field is populated for a
create external table query lineage. Also, modified the
lineage.test file to include location information for all lineages.

Change-Id: If02b0cc16d52c1956298171628f5737cab62ce9f
Reviewed-on: http://gerrit.cloudera.org:8080/14515
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/service/client-request-state.cc
M be/src/util/lineage-util.h
M common/thrift/CatalogService.thrift
M common/thrift/LineageGraph.thrift
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M testdata/workloads/functional-query/queries/QueryTest/lineage.test
M tests/custom_cluster/test_lineage.py
7 files changed, 882 insertions(+), 449 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: If02b0cc16d52c1956298171628f5737cab62ce9f
Gerrit-Change-Number: 14515
Gerrit-PatchSet: 3
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: radford nguyen <ra...@gmail.com>

[Impala-ASF-CR] IMPALA-9070: Include table location in lineage for 'CREATE EXTERNAL TABLE' DDL.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.

Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14515 )

Change subject: IMPALA-9070: Include table location in lineage for 'CREATE EXTERNAL TABLE' DDL.
......................................................................


Patch Set 2: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If02b0cc16d52c1956298171628f5737cab62ce9f
Gerrit-Change-Number: 14515
Gerrit-PatchSet: 2
Gerrit-Owner: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Anurag Mantripragada <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: radford nguyen <ra...@gmail.com>
Gerrit-Comment-Date: Mon, 21 Oct 2019 05:59:24 +0000
Gerrit-HasComments: No