Daniel Vanko has posted comments on this change. ( http://gerrit.cloudera.org:8080/24137 )
Change subject: IMPALA-13810: Generate lineage records for UPDATE and MERGE statements ...................................................................... Patch Set 15: (11 comments) Thanks for the refactoring recommendation, I've done it in PS13. PS14: I added a filter for Kudu's PK columns and swapped the order of LINEAGE and QUERY sections in the new test files, so they're more readable and mirror the QUERY → RESULTS / TYPES / ERRORS convention used everywhere else. http://gerrit.cloudera.org:8080/#/c/24137/12/fe/src/main/java/org/apache/impala/planner/Planner.java File fe/src/main/java/org/apache/impala/planner/Planner.java: http://gerrit.cloudera.org:8080/#/c/24137/12/fe/src/main/java/org/apache/impala/planner/Planner.java@245 PS12, Line 245: return fragments; : } : computeColumnLineage(rootFragment); : } : : > The current two-step logic seems fragile to me (and hard to read), consider Done http://gerrit.cloudera.org:8080/#/c/24137/12/fe/src/main/java/org/apache/impala/planner/Planner.java@368 PS12, Line 368: sAutoInc) { > Add precondition check that it is an Iceberg table. Done http://gerrit.cloudera.org:8080/#/c/24137/12/fe/src/main/java/org/apache/impala/planner/Planner.java@371 PS12, Line 371: + "mismatch", kuduUpdateColIdxs.size(), o > This labels all columns, while addTargetColumnLabelsForUpdate() correctly o Done http://gerrit.cloudera.org:8080/#/c/24137/12/fe/src/main/java/org/apache/impala/planner/Planner.java@418 PS12, Line 418: > Should be a precondition check? Or add comment please. Done http://gerrit.cloudera.org:8080/#/c/24137/11/testdata/workloads/functional-query/queries/QueryTest/lineage-iceberg-update-merge.test File testdata/workloads/functional-query/queries/QueryTest/lineage-iceberg-update-merge.test: http://gerrit.cloudera.org:8080/#/c/24137/11/testdata/workloads/functional-query/queries/QueryTest/lineage-iceberg-update-merge.test@353 PS11, Line 353: "user": "dvanko", : "timestamp": 1775747145, : "endTime": 1775747146, : "edges": [ : { : "sources": [ : 0 : ], : > Ack. We should also have a similar self-assignment for a MERGE statement, a Done, see PS13, line 1107-1176. http://gerrit.cloudera.org:8080/#/c/24137/11/testdata/workloads/functional-query/queries/QueryTest/lineage-iceberg-update-merge.test@810 PS11, Line 810: "edgeType": "PROJECTION" : }, : { : "sources": [ : 2, : 3 : ], : "targets": [ : > Regular updates don't produce these self-assignments. These generate noise, Yes, it's filtered out now. http://gerrit.cloudera.org:8080/#/c/24137/11/testdata/workloads/functional-query/queries/QueryTest/lineage-iceberg-update-merge.test@826 PS11, Line 826: "vertexType": "COLUMN", : "vertexId": "lineage_dml_test_db.iceberg_non_partitioned.user", : "metadata": { : "tableName": "lineage_dml_test_db.iceberg_non_partitioned", : "tableType": "iceberg", : "tableCreateTime": 1776869076 : } : }, : { : "id": 1, : "vertexType": "COLUMN", : "vertexId": "functional_parquet.iceberg_non_partitioned.user", : "metadata": { : "tableName": "functional_parquet.iceberg_non_partitioned", : "tableType": "iceberg", : "tableCreateTime": 1763994561 : } > These are also noise only. Done http://gerrit.cloudera.org:8080/#/c/24137/12/testdata/workloads/functional-query/queries/QueryTest/lineage-iceberg-update-merge.test File testdata/workloads/functional-query/queries/QueryTest/lineage-iceberg-update-merge.test: http://gerrit.cloudera.org:8080/#/c/24137/12/testdata/workloads/functional-query/queries/QueryTest/lineage-iceberg-update-merge.test@704 PS12, Line 704: > Can we have MERGE test with Done http://gerrit.cloudera.org:8080/#/c/24137/12/testdata/workloads/functional-query/queries/QueryTest/lineage-iceberg-update-merge.test@705 PS12, Line 705: merge into lineage_dml_test_ > Can we have tests for MERGE statement with multiple INSERT branches with di Done http://gerrit.cloudera.org:8080/#/c/24137/11/testdata/workloads/functional-query/queries/QueryTest/lineage-kudu-update.test File testdata/workloads/functional-query/queries/QueryTest/lineage-kudu-update.test: http://gerrit.cloudera.org:8080/#/c/24137/11/testdata/workloads/functional-query/queries/QueryTest/lineage-kudu-update.test@418 PS11, Line 418: "user": "dvanko", : "timestamp": 1775747174, : "endTime": 1775747174, : "edges": [ : { : " > Is it possible to filter this out? Yeah, filtered out in PS14 http://gerrit.cloudera.org:8080/#/c/24137/12/testdata/workloads/functional-query/queries/QueryTest/lineage-kudu-update.test File testdata/workloads/functional-query/queries/QueryTest/lineage-kudu-update.test: http://gerrit.cloudera.org:8080/#/c/24137/12/testdata/workloads/functional-query/queries/QueryTest/lineage-kudu-update.test@623 PS12, Line 623: ], > Add tests for updating multiple columns, in various column order. Done -- To view, visit http://gerrit.cloudera.org:8080/24137 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icba79f756509438455bfbf3067733f6f29284220 Gerrit-Change-Number: 24137 Gerrit-PatchSet: 15 Gerrit-Owner: Daniel Vanko <[email protected]> Gerrit-Reviewer: Daniel Vanko <[email protected]> Gerrit-Reviewer: Fang-Yu Rao <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Peter Rozsa <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Tue, 07 Jul 2026 19:44:04 +0000 Gerrit-HasComments: Yes
