Skip to content

Conversation

@rok
Copy link
Member

@rok rok commented Aug 22, 2024

This is to resolve #43787

The Opaque extension type implementation for C++ (plus python bindings) was added in #43458, but it was not registered by default, which we should do for canonical extension types (see #43458 (comment))

Additionally, this adds bool8 extension type builds with ARROW_JSON=false as discussed here

Rationale for this change

Canonical types should be registered by default if possible (except e.g. if they can't be compiled due to ARROW_JSON=false).

What changes are included in this PR?

This adds default registration for opaque, changes when bool8 is built and moves all canonical tests under the same test target.

Are these changes tested?

Changes are tested by previously existing tests.

Are there any user-facing changes?

opaue will now be registered by default and bool8 will be present in case ARROW_JSON=false at build time.

@github-actions github-actions bot added awaiting merge Awaiting merge and removed awaiting committer review Awaiting committer review labels Aug 22, 2024
@rok rok force-pushed the register_opaque_extension_by_default branch from 7f83cdf to 78cfb3e Compare August 22, 2024 09:54
@rok rok force-pushed the register_opaque_extension_by_default branch from 78cfb3e to 09fb602 Compare August 22, 2024 11:45
@rok rok force-pushed the register_opaque_extension_by_default branch from 09fb602 to bcb697a Compare August 22, 2024 11:46
@rok
Copy link
Member Author

rok commented Aug 22, 2024

Fixed a minor nit in the cpp/src/arrow/extension_type.cc, will merge if CI passes.

@rok rok merged commit b4f7efe into apache:main Aug 22, 2024
@rok rok removed the awaiting merge Awaiting merge label Aug 22, 2024
@rok rok deleted the register_opaque_extension_by_default branch August 22, 2024 12:45
@conbench-apache-arrow
Copy link

After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit b4f7efe.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 11 possible false positives for unstable benchmarks that are known to sometimes produce them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[C++] Register the new Opaque extension type by default

3 participants