Fredy Wijaya 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 17: (2 comments) http://gerrit.cloudera.org:8080/#/c/13399/17/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/17/fe/src/main/java/org/apache/impala/analysis/ColumnLineageGraph.java@338 PS17, Line 338: if (v != null) { > can you add comments explaining under what circumstances there would be mis This will never happen, just being defensive about it. I can remove it. http://gerrit.cloudera.org:8080/#/c/13399/17/fe/src/main/java/org/apache/impala/analysis/ColumnLineageGraph.java@726 PS17, Line 726: // We want to use the same Vertex references when building the edge. > can you explain why? So in the unit test, we're modifying the timestamp in the vertices due to dynamic nature of the timestamp. However I didn't modify the vertex instances in the MultiEdge. The normal and easier way is to simply use the same Vertex instances in both Set<Vertex> and MultiEdge. -- 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: 17 Gerrit-Owner: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Anonymous Coward (498) Gerrit-Reviewer: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Comment-Date: Fri, 31 May 2019 21:24:11 +0000 Gerrit-HasComments: Yes