Paul Rogers has posted comments on this change. ( http://gerrit.cloudera.org:8080/11704 )
Change subject: IMPALA-7715: [DOCS] Better descriptions for conditional functions ...................................................................... Patch Set 2: (4 comments) Thanks for making these changes so quickly! A few minor suggestions. http://gerrit.cloudera.org:8080/#/c/11704/2/docs/topics/impala_conditional_functions.xml File docs/topics/impala_conditional_functions.xml: http://gerrit.cloudera.org:8080/#/c/11704/2/docs/topics/impala_conditional_functions.xml@341 PS2, Line 341: <codeph>FALSE</codeph> if the expression is true or <codeph>NULL</codeph>. Should the other refs to boolean constants be called out? "is false" --> "is <codeph>FALSE</codeph>"? Also for true? http://gerrit.cloudera.org:8080/#/c/11704/2/docs/topics/impala_conditional_functions.xml@345 PS2, Line 345: </p> After reading your other change, it appears we support IS FALSE. So, we might add: "Same as the IS FALSE operator." http://gerrit.cloudera.org:8080/#/c/11704/2/docs/topics/impala_conditional_functions.xml@445 PS2, Line 445: <codeph>FALSE</codeph> if the expression is false or <codeph>NULL</codeph>. "an expression" --> "the expression". Code font for true and false. http://gerrit.cloudera.org:8080/#/c/11704/2/docs/topics/impala_conditional_functions.xml@449 PS2, Line 449: </p> Add comment "Is same as the IS TRUE operator." Maybe also do this for "isnottrue() - IS NOT TRUE" and "isnotfalse() - IS NOT FALSE" -- To view, visit http://gerrit.cloudera.org:8080/11704 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5cc90d62645730d2674bcb3af614863aa92b92f6 Gerrit-Change-Number: 11704 Gerrit-PatchSet: 2 Gerrit-Owner: Alex Rodoni <arod...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Paul Rogers <par0...@yahoo.com> Gerrit-Comment-Date: Wed, 17 Oct 2018 01:28:18 +0000 Gerrit-HasComments: Yes
