Good point. bitWidth would be more consistent in that the representation is integer-based.
On Mon, Jun 22, 2020 at 9:20 AM Antoine Pitrou <[email protected]> wrote: > > > Actually, just a nitpick: do we want "bitWidth" (as in Int, TimeUnit), > or "byteWidth" (as in FixedSizeBinary)? > > > > Le 22/06/2020 à 16:12, Wes McKinney a écrit : > > I added a couple of additional comments to the PR -- I figure these > > can be wordsmithed further if there is consensus about adding the new > > table field. If there are no objections, I will start a vote in the > > next 24 hours or so. > > > > Thanks > > > > On Wed, Jun 3, 2020 at 9:21 AM Antoine Pitrou <[email protected]> wrote: > >> > >> > >> Sounds good to me. > >> > >> Regards > >> > >> Antoine. > >> > >> > >> On Mon, 1 Jun 2020 17:47:38 -0500 > >> Wes McKinney <[email protected]> wrote: > >>> I mentioned this on the recent sync call and opened > >>> > >>> https://issues.apache.org/jira/browse/ARROW-8985 > >>> > >>> I believe at some point that Arrow may need to be used to transport > >>> decimal widths different from 128 bits. For example systems like > >>> Apache Kudu have 32-bit and 64-bit decimals. Computational code may > >>> immediately promote small decimals, but it's valuable to be able to > >>> transfer and represent the data as is rather than forcing an > >>> up-promotion even for low-precision decimal data. > >>> > >>> In order to allow for this work to possibly happen in the future > >>> without requiring a new value be added to the "Type" Flatbuffers > >>> union, I propose to add a "byteWidth" field with default value 16 to > >>> the existing Decimal type. Here is a patch with this change: > >>> > >>> https://github.com/apache/arrow/pull/7321 > >>> > >>> To make the forward compatibility issue clear, if this field is not > >>> added then current library versions would not be able to perceive the > >>> absence of the field, this making it unsafe for future library > >>> versions to annotate anything other than 16-byte decimals with this > >>> metadata. > >>> > >>> As part of adopting this change, we would want to add assertions to > >>> the existing libraries to check that the byteWidth is indeed 16 and > >>> either throwing an exception or passing through the data as > >>> FixedSizeBinary otherwise. > >>> > >>> Thanks, > >>> Wes > >>> > >> > >> > >>
