Matthew Jacobs has posted comments on this change. Change subject: IMPALA-1731,IMPALA-3868: Float values are not parsed correctly ......................................................................
Patch Set 3: (5 comments) http://gerrit.cloudera.org:8080/#/c/3622/3//COMMIT_MSG Commit Message: PS3, Line 12: strings with trailing whitespace, right? http://gerrit.cloudera.org:8080/#/c/3622/3/be/src/util/string-parser.h File be/src/util/string-parser.h: PS3, Line 438: else if (!decimal && s[i] == '.') { : decimal = true; : } I think this may allow multiple dots if the following character is e or E, e.g. "0..e", can you test that case? You can move the check for !decimal inside this elif block and fail if decimal is already true. PS3, Line 443: first probably easier to read if this is just 1 and 'first' is removed http://gerrit.cloudera.org:8080/#/c/3622/3/testdata/workloads/functional-query/queries/QueryTest/exprs.test File testdata/workloads/functional-query/queries/QueryTest/exprs.test: PS3, Line 2459: cast('inf' as double), cast('InFinity' as float), please also add positive cases for 'inf ' and 'infinity ' w/ trailing whitespace Line 2467: # IMPALA-1731: test parsing NaN values same about trailing whitespace -- To view, visit http://gerrit.cloudera.org:8080/3622 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9e17d0f051b300a22a520ce34e276c2d4460d35e Gerrit-PatchSet: 3 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Attila Jeges <[email protected]> Gerrit-Reviewer: Attila Jeges <[email protected]> Gerrit-Reviewer: Jim Apple <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-HasComments: Yes
