Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: uuid types in data structures #79

Merged
merged 1 commit into from
Jan 8, 2025

Conversation

noaccOS
Copy link
Contributor

@noaccOS noaccOS commented Jan 3, 2025

the current implementation assumes xandra always returns strings and avoids loading uuids itself.

this usually works, but has a few drawbacks:

  • we're forced to use embedded loaders for data structures, which should not be needed as the structures themselves declare their loaders
  • these embedded loaders perform the UUID load themselves, attempting at loading a string UUID and resulting in an exception

By requiring xandra to send us UUIDs in binary format and performing the loading ourselves, we avoid the issue above.

The drawback of this is that it is no longer possible to request UUIDs in binary format to Xandra, but this did not work for values nested inside maps/sets even before this refactor

the current implementation assumes xandra always returns strings and
avoids loading uuids itself.

this usually works, but has a few drawbacks:
- we're forced to use embedded loaders for data structures, which should
  not be needed as the structures themselves declare their loaders
- these embedded loaders perform the UUID load themselves, attempting at
  loading a string UUID and resulting in an exception

By requiring xandra to send us UUIDs in binary format and performing the
loading ourselves, we avoid the issue above.

The drawback of this is that it is no longer possible to request UUIDs
in binary format to Xandra, but this did not work for values nested
inside maps/sets even before this refactor
@vinniefranco vinniefranco merged commit b498c2b into vinniefranco:main Jan 8, 2025
4 checks passed
@noaccOS noaccOS deleted the push-ppztktulqtkz branch January 30, 2025 08:56
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.

2 participants