Skip to content

Conversation

bodduv
Copy link
Contributor

@bodduv bodduv commented Aug 22, 2025

What's Changed

This PR moves the UUID implementation from tests to appropriate locations so downstream projects can use them. This allows to avoid downstream projects to have their own custom implementation for a well-understood data type. A few modifications, such as the use ArrowBuf, are proposed.

Closes #823 .

@bodduv bodduv changed the title Make UUID implementation public Make UUID implementation publicly available for downstream projects Aug 22, 2025
@bodduv bodduv changed the title Make UUID implementation publicly available for downstream projects Make UUID implementation available for downstream projects Aug 22, 2025
Copy link
Member

@lidavidm lidavidm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The UUID type here was never meant to be a real type, it was implemented solely for testing things and doesn't match the UUID canonical extension type. If there's a desire for a UUID type, please open a new issue and implement the canonical extension type instead.

@bodduv
Copy link
Contributor Author

bodduv commented Aug 23, 2025

@lidavidm thank you for the comment! I can open an issue and work on this. I have a couple of questions:

  • would the canonical extension type implementation for UUID be considerably different the current PR proposes? Are there any concerns regarding the current implementation using ExtensionType, ExtensionHolder and ExtensionVector (apart from the fact that the implementation is moved and modified)?
  • in case we add the canonical extension type for UUID, should we keep UuidType and other relevant code in tests?
    (tagging @wgtmac too)

@lidavidm
Copy link
Member

It's possible they are the same, but at the very least the extension name here is wrong, and I would prefer it be clearly documented and in its own namespace, and for everything to be reviewed with that in mind instead of just moving test code into a feature that has to be supported long-term.

@bodduv bodduv closed this Aug 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make UUID extension type publicly available
2 participants