[C++] ExtensionType doesn't define default bit_width() and byte_width() #41353
Comments
felipecrv
added a commit
that referenced
this issue
Apr 23, 2024
…erms of the storage type (#41354) ### Rationale for this change Users and other classes within Arrow itself (e.g. array builders) expect extension types to behave like their underlying storage type. As it is now, `ExtensionType::bit_width()` is the default `DataType::bit_width()` implementation which returns `-1`. It should return the storage type's bit-width. ### What changes are included in this PR? Definition of `ExtensionType::bit_width/byte_width` functions. ### Are these changes tested? Tests added and confirmed to fail prior to these changes. ### Are there any user-facing changes? `ExtensionType` now define `bit_width` and `byte_width` according to their storage type. * GitHub Issue: #41353 Authored-by: Felipe Oliveira Carvalho <felipekde@gmail.com> Signed-off-by: Felipe Oliveira Carvalho <felipekde@gmail.com>
|
Issue resolved by pull request 41354 |
zanmato1984
pushed a commit
to zanmato1984/arrow
that referenced
this issue
Apr 24, 2024
…e in terms of the storage type (apache#41354) ### Rationale for this change Users and other classes within Arrow itself (e.g. array builders) expect extension types to behave like their underlying storage type. As it is now, `ExtensionType::bit_width()` is the default `DataType::bit_width()` implementation which returns `-1`. It should return the storage type's bit-width. ### What changes are included in this PR? Definition of `ExtensionType::bit_width/byte_width` functions. ### Are these changes tested? Tests added and confirmed to fail prior to these changes. ### Are there any user-facing changes? `ExtensionType` now define `bit_width` and `byte_width` according to their storage type. * GitHub Issue: apache#41353 Authored-by: Felipe Oliveira Carvalho <felipekde@gmail.com> Signed-off-by: Felipe Oliveira Carvalho <felipekde@gmail.com>
raulcd
pushed a commit
that referenced
this issue
Apr 29, 2024
…erms of the storage type (#41354) ### Rationale for this change Users and other classes within Arrow itself (e.g. array builders) expect extension types to behave like their underlying storage type. As it is now, `ExtensionType::bit_width()` is the default `DataType::bit_width()` implementation which returns `-1`. It should return the storage type's bit-width. ### What changes are included in this PR? Definition of `ExtensionType::bit_width/byte_width` functions. ### Are these changes tested? Tests added and confirmed to fail prior to these changes. ### Are there any user-facing changes? `ExtensionType` now define `bit_width` and `byte_width` according to their storage type. * GitHub Issue: #41353 Authored-by: Felipe Oliveira Carvalho <felipekde@gmail.com> Signed-off-by: Felipe Oliveira Carvalho <felipekde@gmail.com>
tolleybot
pushed a commit
to tmct/arrow
that referenced
this issue
May 2, 2024
…e in terms of the storage type (apache#41354) ### Rationale for this change Users and other classes within Arrow itself (e.g. array builders) expect extension types to behave like their underlying storage type. As it is now, `ExtensionType::bit_width()` is the default `DataType::bit_width()` implementation which returns `-1`. It should return the storage type's bit-width. ### What changes are included in this PR? Definition of `ExtensionType::bit_width/byte_width` functions. ### Are these changes tested? Tests added and confirmed to fail prior to these changes. ### Are there any user-facing changes? `ExtensionType` now define `bit_width` and `byte_width` according to their storage type. * GitHub Issue: apache#41353 Authored-by: Felipe Oliveira Carvalho <felipekde@gmail.com> Signed-off-by: Felipe Oliveira Carvalho <felipekde@gmail.com>
vibhatha
pushed a commit
to vibhatha/arrow
that referenced
this issue
May 25, 2024
…e in terms of the storage type (apache#41354) ### Rationale for this change Users and other classes within Arrow itself (e.g. array builders) expect extension types to behave like their underlying storage type. As it is now, `ExtensionType::bit_width()` is the default `DataType::bit_width()` implementation which returns `-1`. It should return the storage type's bit-width. ### What changes are included in this PR? Definition of `ExtensionType::bit_width/byte_width` functions. ### Are these changes tested? Tests added and confirmed to fail prior to these changes. ### Are there any user-facing changes? `ExtensionType` now define `bit_width` and `byte_width` according to their storage type. * GitHub Issue: apache#41353 Authored-by: Felipe Oliveira Carvalho <felipekde@gmail.com> Signed-off-by: Felipe Oliveira Carvalho <felipekde@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the enhancement requested
ExtensionTypeshould implementbit_widthandbyte_widthby delegating the call to the storage type.Component(s)
C++
The text was updated successfully, but these errors were encountered: