Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/21125 )
Change subject: IMPALA-12611: Add support to MAP type Iceberg Metadata table columns ...................................................................... Patch Set 8: (4 comments) http://gerrit.cloudera.org:8080/#/c/21125/8/be/src/exec/iceberg-metadata/iceberg-metadata-scanner.cc File be/src/exec/iceberg-metadata/iceberg-metadata-scanner.cc: http://gerrit.cloudera.org:8080/#/c/21125/8/be/src/exec/iceberg-metadata/iceberg-metadata-scanner.cc@156 PS8, Line 156: jobject map_entry; Shouldn't we release 'map_entry' at the end of this function? http://gerrit.cloudera.org:8080/#/c/21125/8/be/src/exec/iceberg-metadata/iceberg-row-reader.cc File be/src/exec/iceberg-metadata/iceberg-row-reader.cc: http://gerrit.cloudera.org:8080/#/c/21125/8/be/src/exec/iceberg-metadata/iceberg-row-reader.cc@270 PS8, Line 270: env->DeleteLocalRef(collection_scanner); I think we can leak memory if any of the RETURN_IF_ERROR or RETURN_IF_CANCELLED returns from the function. Would it be possible to wrap these globalrefs into some custom object that we write and then we can release the memory in the desctructor? http://gerrit.cloudera.org:8080/#/c/21125/8/be/src/exec/iceberg-metadata/iceberg-row-reader.cc@270 PS8, Line 270: DeleteLocalRef Isn't 'collection_scanner' a GlobalRef? We call DeleteLocalRef here so I'm a bit confused :) http://gerrit.cloudera.org:8080/#/c/21125/8/be/src/exec/iceberg-metadata/iceberg-row-reader.cc@288 PS8, Line 288: env->DeleteLocalRef(item); Same comment about leaking memory -- To view, visit http://gerrit.cloudera.org:8080/21125 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8a8b3a574ca45c893315c3b41b33ce4e0eff865a Gerrit-Change-Number: 21125 Gerrit-PatchSet: 8 Gerrit-Owner: Daniel Becker <daniel.bec...@cloudera.com> Gerrit-Reviewer: Daniel Becker <daniel.bec...@cloudera.com> Gerrit-Reviewer: Gabor Kaszab <gaborkas...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Noemi Pap-Takacs <npaptak...@cloudera.com> Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Comment-Date: Tue, 02 Apr 2024 11:35:08 +0000 Gerrit-HasComments: Yes
