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

Bug fix: Add matching case for unmarshalling nil uuid column #235

Merged
merged 1 commit into from
Aug 20, 2024

Conversation

izenhaim
Copy link

Schemagen generates structs as [16]byte for uuid and timeuuid columns.

when a time/uuid column has nil value, the switch case will not match with *[]byte and returns the following error:

can not unmarshal X timeuuid into *[16]uint8

to solve this we can change Schemagen, or add the case here. personally I belive changing Schemagen has more cost and mapping UUIDs to [16]byte is better than mapping to []byte.

Schemagen generates structs as [16]byte for uuid and timeuuid columns. 

when a time/uuid column has nil value, the switch case will not match with *[]byte and returns the following error:

`can not unmarshal X timeuuid into *[16]uint8`

to solve this we can change Schemagen, or add the case here. personally I belive changing Schemagen has more cost and mapping UUIDs to [16]byte is better than mapping to []byte.
@izenhaim izenhaim changed the title Bug fix: Add matching case for nil uuid column Bug fix: Add matching case for unmarshalling nil uuid column Aug 20, 2024
@izenhaim
Copy link
Author

@sylwiaszunejko @dkropachev
I'm sorry for mentioning you, since this is causing a lot of problems for us, I wanted to get your opinion on it sooner.
Thanks.

@dkropachev
Copy link
Collaborator

@sylwiaszunejko @dkropachev I'm sorry for mentioning you, since this is causing a lot of problems for us, I wanted to get your opinion on it sooner. Thanks.

@izenhaim , thanks for you contribution, my bad we have not picked it up yearlier.

@dkropachev dkropachev added the bug label Aug 20, 2024
@dkropachev dkropachev merged commit 00277be into scylladb:master Aug 20, 2024
1 check passed
@izenhaim
Copy link
Author

Glad to be of help!
will you release a version as well?

@izenhaim izenhaim deleted the patch-1 branch August 20, 2024 10:11
@mykaul
Copy link

mykaul commented Aug 20, 2024

We should have a test for it (to all marshaling cases, I assume)

@dkropachev
Copy link
Collaborator

We should have a test for it (to all marshaling cases, I assume)

#237

@dkropachev
Copy link
Collaborator

Glad to be of help! will you release a version as well?

Releasing it take a one week, meanwhile you can use fixed version as the following:
replace github.com/gocql/gocql => github.com/scylladb/gocql v1.14.3-0.20240820095829-00277be38708

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants