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: (1 comment) 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 see. Thanks for the details. So those conditions do not determine ELSE i. Okay. Thanks for the explanation. Since parse_number() parses a prefix of max length from input 'p' to a double, we would have to make sure it can do so with our input 's' even when we take the ELASE branch. For example, if the input 's' is "-" of length 1, will we crash in parse_numer() at line 1141? 1133 WARN_UNUSED 1134 really_inline const char * parse_number(const char *p, double *outDouble) { 1135 const char *pinit = p; 1136 bool found_minus = (*p == '-'); 1137 bool negative = false; 1138 if (found_minus) { 1139 ++p; 1140 negative = true; 1141 if (!is_integer(*p)) { // a negative sign must be followed by an integer 1142 return nullptr; 1143 } 1144 } It sounds like we would have to convert to null-terminating string first, unless we modify parse_number()? -- 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: Mon, 12 Jul 2021 13:56:37 +0000 Gerrit-HasComments: Yes
