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 8: (3 comments) http://gerrit.cloudera.org:8080/#/c/17389/7/be/src/util/string-parser-test.cc File be/src/util/string-parser-test.cc: http://gerrit.cloudera.org:8080/#/c/17389/7/be/src/util/string-parser-test.cc@535 PS7, Line 535: TestStringToFloatPreprocess(".43256e4", "0.43256e4"); > Preprocess function doesn't handle sign either '+' or '-', so have added it I see. Done. http://gerrit.cloudera.org:8080/#/c/17389/7/be/src/util/string-parser.h File be/src/util/string-parser.h: http://gerrit.cloudera.org:8080/#/c/17389/7/be/src/util/string-parser.h@508 PS7, Line 508: s[len]) > I didn't understand the conditions above, but if s ='99' (which satisfies c The idea is to find the conditions to go to the ELSE branch quickly (without looking ahead too much). The JSON specs on numeric values are as follows (from https://datatracker.ietf.org/doc/html/rfc7159), on which the fast_double_parser is based. Numeric values that cannot be represented in the grammar below (such as Infinity and NaN) are not permitted. number = [ minus ] int [ frac ] [ exp ] decimal-point = %x2E ; . digit1-9 = %x31-39 ; 1-9 e = %x65 / %x45 ; e E exp = e [ minus / plus ] 1*DIGIT frac = decimal-point 1*DIGIT int = zero / ( digit1-9 *DIGIT ) minus = %x2D ; - plus = %x2B ; + zero = %x30 ; 0 Thus, the conditions to do so are 1). minus digit1-9 2). digit1-9 3). 0 frac Sorry I did not spell out these conditions precisely in the first attempt. http://gerrit.cloudera.org:8080/#/c/17389/7/be/src/util/string-parser.h@541 PS7, Line 541: return (T)(negative ? -val : val); > It will be reached for UNDERFLOW and OVERFLOW. 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: 8 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: Sun, 11 Jul 2021 12:55:30 +0000 Gerrit-HasComments: Yes
