Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/19507 )
Change subject: IMPALA-11854: ImpalaStringWritable's underlying array can't be changed in UDFs ...................................................................... Patch Set 15: (2 comments) http://gerrit.cloudera.org:8080/#/c/19507/14/fe/src/main/java/org/apache/impala/hive/executor/ImpalaBytesWritable.java File fe/src/main/java/org/apache/impala/hive/executor/ImpalaBytesWritable.java: http://gerrit.cloudera.org:8080/#/c/19507/14/fe/src/main/java/org/apache/impala/hive/executor/ImpalaBytesWritable.java@35 PS14, Line 35: super.set(bytes, 0, bytes.length); > Changed ordering, setCapacity is called first. Do we actually need the setCapacity() call? Calling set() will handle increasing capacity if needed, so calling setCapacity() is only useful if the new size is smaller than the old one - we can reclaim memory then but at the expense of an allocation. Is it worth it? Maybe we should only reset the capacity if the size is significantly smaller? http://gerrit.cloudera.org:8080/#/c/19507/15/java/test-hive-udfs/src/main/java/org/apache/impala/CachedWritablesUdf.java File java/test-hive-udfs/src/main/java/org/apache/impala/CachedWritablesUdf.java: http://gerrit.cloudera.org:8080/#/c/19507/15/java/test-hive-udfs/src/main/java/org/apache/impala/CachedWritablesUdf.java@77 PS15, Line 77: return new BytesWritable(target.getBytes(), target.getLength()); Can't we return 'target' here? -- To view, visit http://gerrit.cloudera.org:8080/19507 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifb28bd0dce7b0482c7abe1f61f245691fcbfe212 Gerrit-Change-Number: 19507 Gerrit-PatchSet: 15 Gerrit-Owner: Peter Rozsa <pro...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Daniel Becker <daniel.bec...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Peter Rozsa <pro...@cloudera.com> Gerrit-Comment-Date: Fri, 03 Mar 2023 10:18:48 +0000 Gerrit-HasComments: Yes
