Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21782 )
Change subject: IMPALA-12876: Add catalogVersion and loaded timestamp in query profiles ...................................................................... Patch Set 4: (3 comments) http://gerrit.cloudera.org:8080/#/c/21782/3/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCtasTarget.java File fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCtasTarget.java: http://gerrit.cloudera.org:8080/#/c/21782/3/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergCtasTarget.java@349 PS3, Line 349: > I agree with this suggestion since the ctas target should always be a table Moved this to CtasTargetTable but still keep returning 0 for simplicity. This is generated by the Analyzer and doesn't exist in StmtTableCache since the table doesn't exist at the beginning. Showing the catalog version of the just-created target table needs additional code. I think it's not that useful since there are no concurrent INSERTs on the table. So we don't need to reason about what version of it is used. BTW, if the target table already exists and we use CTAS with IF NOT EXISTS, StmtTableCache will be able to load the table so it appears in the profile as well. The INSERT part of the CTAS is skipped since the table already exists. I change "Table Versions" to "Original Table Versions" to reflect that those are catalog versions before the statement is executed. We can add the catalog version of the target table after DDL/DML statements finish in future JIRAs. http://gerrit.cloudera.org:8080/#/c/21782/3/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java File fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java: http://gerrit.cloudera.org:8080/#/c/21782/3/fe/src/main/java/org/apache/impala/catalog/iceberg/IcebergMetadataTable.java@170 PS3, Line 170: > A VirtualTable by definition is not represented in HMS, and so wouldn't hav Done. Moved these to VirtualTable. They are created by the Analyzer and don't exist in the StmtTableCache. So these functions are actually not used. http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/service/Frontend.java File fe/src/main/java/org/apache/impala/service/Frontend.java: http://gerrit.cloudera.org:8080/#/c/21782/2/fe/src/main/java/org/apache/impala/service/Frontend.java@2585 PS2, Line 2585: // Add the catalog versions and loaded timestamps. > I'm not sure about the Thrift/JSON profile. If we cannot use different for A table like ExecSummary adds additional padding. Keeps the current format since I think it's easier to parse and extract info from it. -- To view, visit http://gerrit.cloudera.org:8080/21782 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I94b2fd59ed5aca664d6db4448c61ad21a88a4f98 Gerrit-Change-Number: 21782 Gerrit-PatchSet: 4 Gerrit-Owner: Quanlong Huang <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Comment-Date: Fri, 13 Sep 2024 09:04:02 +0000 Gerrit-HasComments: Yes
