Laurel Hale has posted comments on this change.

Change subject: IMPALA-3603 [DOCS] Document handling of NaN values
......................................................................


Patch Set 2:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/7098/2//COMMIT_MSG
Commit Message:

PS2, Line 13: This information has been added to the subsection
            : "Usage Notes:" in each topic:
            : 
            : "Impala does not evaluate NaN (not a number) values as
            : equal to any other numeric values, including NaN. For
            : example, the following statement, which evaluates
            : equality between two NaN values returns 'false':
> The commit message doesn't need to be this detailed. I would just add a  an
okay.


PS2, Line 23: For patch set #2
> The commit message shouldn't reference the revisions of the patch. It's mea
Thanks.


http://gerrit.cloudera.org:8080/#/c/7098/2/docs/shared/impala_common.xml
File docs/shared/impala_common.xml:

PS2, Line 2298:       
> Extra whitespace. Remove.
Done


PS2, Line 2300: (not a number)
> Redundant.
I disagree about including "(not a number)." It's a documentation best practice 
to always provide what an acronym stands for on first use in a topic to avoid 
ambiguity.

I agree with removing "values" and will do that.


PS2, Line 2302: returns
> Consider "evaluates to" rather than "returns"...I slightly prefer the forme
I think the subordinate clause is important for clarity and so will leave as is.


PS2, Line 2301: , which evaluates equality
              :         between two NaN values
> Redundant.
I disagree. Will leave it in.


http://gerrit.cloudera.org:8080/#/c/7098/2/docs/topics/impala_double.xml
File docs/topics/impala_double.xml:

PS2, Line 80:     
> Extra whitespace.
Done


http://gerrit.cloudera.org:8080/#/c/7098/2/docs/topics/impala_float.xml
File docs/topics/impala_float.xml:

PS2, Line 74:     
> Extra whitespace.
Done


PS2, Line 76:     
> Extra whitespace.
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/7098
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id9485b6790d58fafdae32332d2634cbe893d7fb0
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Laurel Hale <lau...@cloudera.com>
Gerrit-Reviewer: John Russell <jruss...@cloudera.com>
Gerrit-Reviewer: Laurel Hale <lau...@cloudera.com>
Gerrit-Reviewer: Will Berkeley <wdberke...@gmail.com>
Gerrit-HasComments: Yes

Reply via email to