You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Todd Lipcon (Code Review)" <ge...@cloudera.org> on 2019/06/03 17:37:30 UTC

[Impala-ASF-CR] IMPALA-8564: Add table/view create time in the lineage graph

Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/13399 )

Change subject: IMPALA-8564: Add table/view create time in the lineage graph
......................................................................


Patch Set 18:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/13399/18/fe/src/main/java/org/apache/impala/analysis/ColumnLineageGraph.java
File fe/src/main/java/org/apache/impala/analysis/ColumnLineageGraph.java:

http://gerrit.cloudera.org:8080/#/c/13399/18/fe/src/main/java/org/apache/impala/analysis/ColumnLineageGraph.java@787
PS18, Line 787:    * This is only for testing. It does not check for user and timestamp fields.
since it's only for testing, could we change this to not override Object.equals, but instead be named something like 'equalForTests()'? Given that .equals() is called implicitly in a lot of cases, this seems like a ticking time bomb that someone uses it and gets confused by the semantics. (same goes for the above)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If4f578d7b299a76c30323b10a883ba32f8713d82
Gerrit-Change-Number: 13399
Gerrit-PatchSet: 18
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (498)
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Mon, 03 Jun 2019 17:37:30 +0000
Gerrit-HasComments: Yes