KalleOlaviNiemitalo commented on PR #2282:
URL: https://github.com/apache/avro/pull/2282#issuecomment-1751599774

   I don't intend this comment to delay the merging of this pull request.
   
   So far, all Avro logical types annotate `fixed`, `bytes`, `string`, `int`, 
or `long` types.  From 
<https://github.com/apache/avro/pull/2282#discussion_r1349457370>, it occurs to 
me that "big-decimal" might be reasonable to use with a `record` type, similar 
to a [CBOR decimal fraction (tag number 
4)](https://www.rfc-editor.org/rfc/rfc8949.html#fractions):
   
   ``` JSON
   {
       "type": "record",
       "namespace": "apache.avro.test",
       "name": "RecordOfTwoBigDecimals",
       "fields": [
           {
               "name": "dec1",
               "type": {
                   "type": "record",
                   "name": "apache.avro.BigDecimal",
                   "fields": [
                       {
                           "name": "unscaled",
                           "type": "bytes"
                       },
                       {
                           "name": "scale",
                           "type": "int"
                       }
                   ],
                   "logicalType": "big-decimal"
               }
           },
           {
               "name": "dec2",
               "type": "apache.avro.BigDecimal"
           }
       ]
   }
   ```
   
   * Pro: Would make the binary encoding a little smaller, as the length field 
of the "bytes" value around the "big-decimal" logical value would be omitted.
   * Pro: This would cause the JSON encoding to have "unscaled" and "scale" as 
separate fields, making it easier to read.
   * Con: The value of the "unscaled" field in JSON could lose precision if a 
library reads it as a `double` (especially in JavaScript).
   * Con: Schemas using "big-decimal" would become more cumbersome to write, as 
the underlying record type would have to be defined.  This could be mitigated 
by having the schema parser define "apache.avro.BigDecimal" implicitly if 
referenced but not locally defined; but if that causes the 
"apache.avro.BigDecimal" definition to become embedded in a schema that is 
uploaded to a schema registry service, then there is a risk that the schema 
might be considered a derivative work of Avro and require Apache legal notices 
to be included.
   * Con: Would require larger changes in the implementations.  The C# library 
does not currently support a named schema having a logical type.
   
   Overall, the difficulties of that approach seem to outweigh the benefits.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to