Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/18091 )
Change subject: IMPALA-11057: Speed up hex_int function ...................................................................... Patch Set 8: (4 comments) http://gerrit.cloudera.org:8080/#/c/18091/8//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/18091/8//COMMIT_MSG@10 PS8, Line 10: streamstring Nit: stringstream. http://gerrit.cloudera.org:8080/#/c/18091/8//COMMIT_MSG@11 PS8, Line 11: However, the initialization of std::stringstream depends on std::locale, Please keep line width to at most 72 in the commit message. http://gerrit.cloudera.org:8080/#/c/18091/8/be/src/exprs/math-functions-ir.cc File be/src/exprs/math-functions-ir.cc: http://gerrit.cloudera.org:8080/#/c/18091/8/be/src/exprs/math-functions-ir.cc@254 PS8, Line 254: ptr[i] = '\0'; Do we actually need to add the '\0' byte? We don't include it on L263. http://gerrit.cloudera.org:8080/#/c/18091/8/be/src/exprs/math-functions-ir.cc@257 PS8, Line 257: for (int k = 0, j = i - 1; k <= j; k++, j--) { Can we use std::reverse() here? -- To view, visit http://gerrit.cloudera.org:8080/18091 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icf2dc7ea64b60b24f1a9e6ab53725acd40ce88ac Gerrit-Change-Number: 18091 Gerrit-PatchSet: 8 Gerrit-Owner: Anonymous Coward <[email protected]> Gerrit-Reviewer: Anonymous Coward <[email protected]> Gerrit-Reviewer: Daniel Becker <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Kurt Deschler <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Comment-Date: Thu, 13 Apr 2023 08:52:03 +0000 Gerrit-HasComments: Yes
