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 #1442: add codecs for UUID type(s) #1496

Merged
merged 9 commits into from
Oct 8, 2024
Merged

Conversation

tatu-at-datastax
Copy link
Contributor

What this PR does:

Adds codec impls, tests (unit, integration) for cql "uuid" and "timeuuid" types.

Which issue(s) this PR fixes:
Fixes #1442

Checklist

  • Changes manually tested
  • Automated Tests added/updated
  • Documentation added/updated
  • CLA Signed: DataStax CLA

@tatu-at-datastax tatu-at-datastax self-assigned this Oct 4, 2024
@@ -48,6 +48,10 @@ public abstract class JSONCodecRegistries {
JSONCodecs.DURATION_FROM_STRING,
JSONCodecs.TIME_FROM_STRING,
JSONCodecs.TIMESTAMP_FROM_STRING,
// UUID codecs
JSONCodecs.UUID_FROM_STRING,
JSONCodecs.TIMEUUID_FROM_STRING,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Although we won't allow creation of "timeuuid" type, we do allow read/write, for which codec needed

Copy link
Contributor

@Yuqi-Du Yuqi-Du Oct 8, 2024

Choose a reason for hiding this comment

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

Sorry, I may miss the context for this one.
Why do we not allow "timeuuid" type?

Copy link
Contributor Author

@tatu-at-datastax tatu-at-datastax Oct 8, 2024

Choose a reason for hiding this comment

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

I am not 100% sure; I think it may be considered confusing thing -- you can store time-based UUIDs in "uuid" column anyway. And it is "Cassandraism" so hiding it may improve user experience.

Still: we can add support easily in future, if we so decide, but right now spreadsheet includes type as unsupported.

@tatu-at-datastax tatu-at-datastax changed the title (WIP) Fix #1442: add codecs for UUID type(s) Fix #1442: add codecs for UUID type(s) Oct 8, 2024
@tatu-at-datastax tatu-at-datastax marked this pull request as ready for review October 8, 2024 22:07
@tatu-at-datastax tatu-at-datastax requested a review from a team as a code owner October 8, 2024 22:07
Copy link
Contributor

@Yuqi-Du Yuqi-Du left a comment

Choose a reason for hiding this comment

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

LGTM, one comment.

@tatu-at-datastax tatu-at-datastax merged commit 7e622bc into main Oct 8, 2024
3 checks passed
@tatu-at-datastax tatu-at-datastax deleted the tatu/1442-codec-uuid branch October 8, 2024 22:35
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.

Data API JsonCodec impl: UUID (uuid, timeuuid)
2 participants