Qifan Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/17389 )
Change subject: IMPALA-10680: Replace StringToFloatInternal using fast_double_parser library ...................................................................... Patch Set 6: Code-Review+1 (4 comments) Looks great! http://gerrit.cloudera.org:8080/#/c/17389/6/be/src/util/string-parser.h File be/src/util/string-parser.h: http://gerrit.cloudera.org:8080/#/c/17389/6/be/src/util/string-parser.h@538 PS6, Line 538: const char* s, int offset, : int le nit. Maybe we could use two arguments (instead of three): char* start and char* end, where 'start' is s + offset, and 'end' is 'start' + len -1 (inclusive) on entry into this function. In the body, we set char* pos = start, and work with *pos and *(pos+1). In this way, we increment on pos only to avoid s+i in several places. http://gerrit.cloudera.org:8080/#/c/17389/5/be/src/util/string-parser.h File be/src/util/string-parser.h: http://gerrit.cloudera.org:8080/#/c/17389/5/be/src/util/string-parser.h@493 PS5, Line 493: ng decimal > Thats a good catch. I have changed it to string and code is much simpler no ack :-). http://gerrit.cloudera.org:8080/#/c/17389/5/be/src/util/string-parser.h@501 PS5, Line 501: > 'pos' is not going to be 'loop constant' which can be pulled out as it need I see. See my comment to the new version of the code. http://gerrit.cloudera.org:8080/#/c/17389/5/be/src/util/string-parser.h@524 PS5, Line 524: esult = PARSE_OVERFLOW; > Actually, when function underflows it doesn't return NaN. It returns 0 inst Done -- To view, visit http://gerrit.cloudera.org:8080/17389 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic105ad38a2fcbf2fb4e8ae8af6d9a8e251a9c141 Gerrit-Change-Number: 17389 Gerrit-PatchSet: 6 Gerrit-Owner: Amogh Margoor <[email protected]> Gerrit-Reviewer: Amogh Margoor <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Qifan Chen <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Fri, 21 May 2021 01:16:18 +0000 Gerrit-HasComments: Yes
