Jim Apple has posted comments on this change. ( http://gerrit.cloudera.org:8080/11075 )
Change subject: IMPALA-4690: [DOCS] More content for CONV() ...................................................................... Patch Set 3: (8 comments) http://gerrit.cloudera.org:8080/#/c/11075/2/docs/topics/impala_math_functions.xml File docs/topics/impala_math_functions.xml: http://gerrit.cloudera.org:8080/#/c/11075/2/docs/topics/impala_math_functions.xml@228 PS2, Line 228: > This is somewhat opaque. What it seems to be doing is interpret -17 as an u This comment was not replied to. http://gerrit.cloudera.org:8080/#/c/11075/2/docs/topics/impala_math_functions.xml@229 PS2, Line 229: <codeph>from_base</codeph> or <codeph>to_base</codeph> is below > The above number returns SO what happens first: conversion to two's complement or conversion to base 2? http://gerrit.cloudera.org:8080/#/c/11075/2/docs/topics/impala_math_functions.xml@249 PS2, Line 249: <codeph>from_base</codeph>, e.g. 3 in base 2 or 'a123' in base 10, FYI: Whitespace changes like this can make it hard to read diffs like this: https://gerrit.cloudera.org/#/c/11075/2..3/docs/topics/impala_math_functions.xml http://gerrit.cloudera.org:8080/#/c/11075/2/docs/topics/impala_math_functions.xml@279 PS2, Line 279: eph>cosh(double a)</codeph> : </dt> > Not sure how to make this clear. A suggestion? My suggestion is that in the function prototypes, you name the numeric argument n and the string argument s. http://gerrit.cloudera.org:8080/#/c/11075/3/docs/topics/impala_math_functions.xml File docs/topics/impala_math_functions.xml: http://gerrit.cloudera.org:8080/#/c/11075/3/docs/topics/impala_math_functions.xml@235 PS3, Line 235: and <codeph>from_base</codeph> is a negative number.</li> How are negative bases interpreted when `a` is a negative number? How about a binary number encoded in two's complement? http://gerrit.cloudera.org:8080/#/c/11075/3/docs/topics/impala_math_functions.xml@240 PS3, Line 240: <li> String representation of -1 in <codeph>to_base</codeph> if nit: "The string representation", here and below. http://gerrit.cloudera.org:8080/#/c/11075/3/docs/topics/impala_math_functions.xml@241 PS3, Line 241: <codeph>to_base</codeph> is negative and thus the nit: comma after "negative" http://gerrit.cloudera.org:8080/#/c/11075/3/docs/topics/impala_math_functions.xml@242 PS3, Line 242: <codeph>a</codeph> argument represents a signed number. </li> elide the "and thus" part, here and below - it's confusing. -- To view, visit http://gerrit.cloudera.org:8080/11075 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4e424fd5a009ff5aa2d35a403e08fcd33c75fec5 Gerrit-Change-Number: 11075 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Rodoni <arod...@cloudera.com> Gerrit-Reviewer: Alex Rodoni <arod...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Jim Apple <jbapple-imp...@apache.org> Gerrit-Comment-Date: Thu, 02 Aug 2018 18:26:17 +0000 Gerrit-HasComments: Yes
