You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2020/10/12 13:51:37 UTC

[GitHub] [arrow] dchigarev commented on pull request #8417: WIP: [C++] Get rid of code duplication in Decimal##bit_width

dchigarev commented on pull request #8417:
URL: https://github.com/apache/arrow/pull/8417#issuecomment-707133198


   @emkornfield thanks for your comment! 
   
   1. I've changed this PR a bit and get rid of declaring API via macroses. I've left it only at declaring decimals at [`type_fwd.h`](https://github.com/dchigarev/arrow/blob/4346e4e7553af0ef053668c24ca32df61a8deeb1/cpp/src/arrow/type_fwd.h#L147-L157) and [`type_traits.h`](https://github.com/dchigarev/arrow/blob/4346e4e7553af0ef053668c24ca32df61a8deeb1/cpp/src/arrow/type_traits.h#L285-L297) because these files [already using](https://github.com/dchigarev/arrow/blob/4346e4e7553af0ef053668c24ca32df61a8deeb1/cpp/src/arrow/type_fwd.h#L183-L202) this pattern to declare alike classes, if this is acceptable I can leave macroses instead of code duplicating at these two files.
   
   2. These changes should not break `BasicDecimal` at Gandiva, because everything that was added to base decimals is [`decimal_meta.h`](https://github.com/dchigarev/arrow/blob/d256/cpp/src/arrow/util/decimal_meta.h) which I guess do not have references to anything that could break Gandiva
   
   Moving forward, I think that it could be useful if we'll declare some interface for `BasicDecimal` and `Decimal` classes, which will be implemented by `BasicDecimal/Decimal128, 256, 64 ...`. For now, amount of methods in BasicDecimal128 and 256 are noticeably different, is it a kind of TODO to implement in 256 all methods that 128 has, or 256-bit decimal will have less methods on purpose? If it is, is there a list of basic methods that any decimal must have and whose declaration could be moved to a decimal interface?
   
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org