Noemi Pap-Takacs has posted comments on this change. ( http://gerrit.cloudera.org:8080/22728 )
Change subject: IMPALA-13738 (Part2): Clean up code in Catalog's table and partition interfaces ...................................................................... Patch Set 5: (1 comment) Patch set 5 is a rebase to resolve the merge conflict. http://gerrit.cloudera.org:8080/#/c/22728/2/fe/src/main/java/org/apache/impala/catalog/FeFsTable.java File fe/src/main/java/org/apache/impala/catalog/FeFsTable.java: http://gerrit.cloudera.org:8080/#/c/22728/2/fe/src/main/java/org/apache/impala/catalog/FeFsTable.java@496 PS2, Line 496: long numFiles = p.getNumFileDescriptors(); > Heh, I think we'll have to agree to disagree here. Valid and good points, thank you, Steve! I agree that Pair is not a nice solution, so I would avoid that. Thank you for giving this thought. I see the benefits of your suggestion, but I think we have to agree to disagree here for the aforementioned reasons. I think if we add a use case specific field (totalCachedBytes) to TResultRowBuilder, it stops being the neutral meta result builder class it is supposed to be. But most importantly, the TResultRow it builds only makes sense with the schema, so I would not separate the row creation from the schema definition. Therefore I would like to keep this function as it is. -- To view, visit http://gerrit.cloudera.org:8080/22728 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8d9c7ba876e39fa4f4d067761f25dc4ecfd55702 Gerrit-Change-Number: 22728 Gerrit-PatchSet: 5 Gerrit-Owner: Noemi Pap-Takacs <[email protected]> Gerrit-Reviewer: Daniel Becker <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Noemi Pap-Takacs <[email protected]> Gerrit-Reviewer: Steve Carlin <[email protected]> Gerrit-Comment-Date: Thu, 24 Apr 2025 11:39:25 +0000 Gerrit-HasComments: Yes
