Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/21038 )
Change subject: IMPALA-12426: Adds the Impala built-in functions prettyprint_duration and prettyprint_memory. ...................................................................... Patch Set 3: (2 comments) Updates from review comments. http://gerrit.cloudera.org:8080/#/c/21038/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21038/2//COMMIT_MSG@8 PS2, Line 8: > Second line should be blank, it's OK to make the title line longer. Done http://gerrit.cloudera.org:8080/#/c/21038/2/be/src/exprs/expr-test.cc File be/src/exprs/expr-test.cc: http://gerrit.cloudera.org:8080/#/c/21038/2/be/src/exprs/expr-test.cc@4353 PS2, Line 4353: TestStringValue("prettyprint_duration(1234)", "1.234us"); > Can you add test with min and max value of BIGINT (-9223372036854775808 and Done Adding these min/max value tests revealed an interesting bug in the prettyprint_duration function. There was an overflow occurring, and thus an incorrect value was returned. The reason is that the input values were being multiplied by 1000 within the PrettyPrint C++ code to convert them from microseconds to nanoseconds and overflowing the int64_t which stored the result. The fix required promoting the input to a double. Since the overflow happened on very large values that are unlikely to ever actually be needed, I modified the function to take nanoseconds as input instead of microseconds. Thus, no multiplication is needed to convert to nanoseconds and overflows do not happen. The max duration that can be pretty printed is 2562047 hours, 47 minutes or around 292 and a half years which is big enough for workload management (which is the intended use of this function). -- To view, visit http://gerrit.cloudera.org:8080/21038 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3e76632ce21ad2ca5df474160338699a542a6913 Gerrit-Change-Number: 21038 Gerrit-PatchSet: 3 Gerrit-Owner: Jason Fehr <[email protected]> Gerrit-Reviewer: Andrew Sherman <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Comment-Date: Fri, 16 Feb 2024 19:30:37 +0000 Gerrit-HasComments: Yes
