Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/10066 )
Change subject: IMPALA-6522: [DOCS] Document Decimal V2 ...................................................................... Patch Set 10: (18 comments) http://gerrit.cloudera.org:8080/#/c/10066/10/docs/topics/impala_decimal.xml File docs/topics/impala_decimal.xml: http://gerrit.cloudera.org:8080/#/c/10066/10/docs/topics/impala_decimal.xml@271 PS10, Line 271: Note that in memory and on disk for binary file formats such as Parquet or Avro, This seems to contradict what L262 says which claims that binary formats will use fewer bytes. http://gerrit.cloudera.org:8080/#/c/10066/10/docs/topics/impala_decimal.xml@280 PS10, Line 280: <b>Precision and scale in arithmetic operations and UNION:</b> I think we should have a separate section for decimal assignments where the implicit decimal->decimal conversions are strict. http://gerrit.cloudera.org:8080/#/c/10066/10/docs/topics/impala_decimal.xml@284 PS10, Line 284: Unlike other numeric types, memory footprint of <codeph>DECIMAL</codeph> can increase as a I don't think this is correct or even needed. If we add two INTs we get a BIGINT, so that also increases the memory footprint. I suggest removing this paragraph. http://gerrit.cloudera.org:8080/#/c/10066/10/docs/topics/impala_decimal.xml@289 PS10, Line 289: For all arithmetic options, the resulting precision is maximum of 38. is at most 38 http://gerrit.cloudera.org:8080/#/c/10066/10/docs/topics/impala_decimal.xml@293 PS10, Line 293: If the precision of the result would be greater than 38, Impala truncates the result from truncates and rounds, right Taras? This is also not quite right for UNION, so I think it's better to have a separate section for decimal assignment. http://gerrit.cloudera.org:8080/#/c/10066/10/docs/topics/impala_decimal.xml@512 PS10, Line 512: Impala enforces strict conversion rules in <codeph>INSERT</codeph> statements or This section seems to be for "decimal assignment" like I mentioned above. http://gerrit.cloudera.org:8080/#/c/10066/10/docs/topics/impala_decimal.xml@523 PS10, Line 523: <codeph>FLOAT</codeph> when necessary even with a loss of precision. It can be loss of precision -> loss of information http://gerrit.cloudera.org:8080/#/c/10066/10/docs/topics/impala_decimal.xml@544 PS10, Line 544: <codeph>DECIMAL</codeph> cannot be implicitly converted to <codeph>DECIMAL</codeph> if I'd phrase this positively, i.e. describe in which cases the conversion is allowed. http://gerrit.cloudera.org:8080/#/c/10066/10/docs/topics/impala_decimal.xml@613 PS10, Line 613: <codeph>DOUBLE</codeph>), Impala raises an error and does not convert the value. does not implicitly convert the value. http://gerrit.cloudera.org:8080/#/c/10066/10/docs/topics/impala_decimal.xml@618 PS10, Line 618: <codeph>DECIMAL</codeph> and other types in <codeph>INSERT</codeph> statements: in decimal assignments like in INSERT and UNION statements or functions like COALESCE() http://gerrit.cloudera.org:8080/#/c/10066/10/docs/topics/impala_decimal.xml@668 PS10, Line 668: expressions to <codeph>DECIMAL</codeph> as long as the overall number of digits and digits Sentence seem wrong. We follow same general procedure here, round from the back or error if there are not enough digits before the decimal point. http://gerrit.cloudera.org:8080/#/c/10066/10/docs/topics/impala_decimal.xml@690 PS10, Line 690: <codeph>DECIMAL</codeph>, an error returns. an error is returned http://gerrit.cloudera.org:8080/#/c/10066/10/docs/topics/impala_decimal.xml@710 PS10, Line 710: Casting any non-numeric value, such as <codeph>'ABC'</codeph>, or <codeph>NULL</codeph> to Casting NULL to a decimal is allowed. http://gerrit.cloudera.org:8080/#/c/10066/10/docs/topics/impala_decimal.xml@724 PS10, Line 724: <b>DECIMAL vs FLOAT or DOUBLE consideration:</b> DECIMAL vs. FLOAT considerations: http://gerrit.cloudera.org:8080/#/c/10066/10/docs/topics/impala_decimal.xml@728 PS10, Line 728: The <codeph>FLOAT</codeph> and <codeph>DOUBLE</codeph> type numbers can cause problems or type numbers -> types http://gerrit.cloudera.org:8080/#/c/10066/10/docs/topics/impala_decimal.xml@732 PS10, Line 732: values for <codeph>GROUP BY</codeph> columns. The <codeph>DECIMAL</codeph> values can help The DECIMAL type http://gerrit.cloudera.org:8080/#/c/10066/10/docs/topics/impala_decimal.xml@879 PS10, Line 879: Any values that do not fit within the new precision and scale are returned as Taras, is this really true? I would have expected that we round or error. http://gerrit.cloudera.org:8080/#/c/10066/10/docs/topics/impala_decimal.xml@934 PS10, Line 934: Because <codeph>DECIMAL</codeph> type has a fixed size, the maximum and average size the DECIMAL type -- To view, visit http://gerrit.cloudera.org:8080/10066 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic436ff80c9ad05cfada97280cd47552879214a3d Gerrit-Change-Number: 10066 Gerrit-PatchSet: 10 Gerrit-Owner: Alex Rodoni <arod...@cloudera.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Alex Rodoni <arod...@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Greg Rahn <gr...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Taras Bobrovytsky <tbobrovyt...@cloudera.com> Gerrit-Comment-Date: Fri, 27 Apr 2018 23:32:33 +0000 Gerrit-HasComments: Yes
