Original code assumed that all string objects in the format of a UUID we... #122

Merged
merged 1 commit into from Oct 28, 2013

Conversation

Projects
None yet
3 participants
@azweb76
Contributor

azweb76 commented Oct 14, 2013

We upgraded to the latest version and our code failed because we store an ID which looks like a UUID, but is text. Can you review the changes and if ok, please merge into master? Thanks, Dan

Original code assumed that all string objects in the format of a UUID…
… were associated with a UUID data type. Replaced with UUID object check.
@devdazed

This comment has been minimized.

Show comment Hide comment
@devdazed

devdazed Oct 15, 2013

Member

I see where this can cause issue, however, the primary concern I have with this is that it may cause issues with those that aren't explicitly using the UUID type, rather using strings. Anyone else want to chime in?

Member

devdazed commented Oct 15, 2013

I see where this can cause issue, however, the primary concern I have with this is that it may cause issues with those that aren't explicitly using the UUID type, rather using strings. Anyone else want to chime in?

@azweb76

This comment has been minimized.

Show comment Hide comment
@azweb76

azweb76 Oct 15, 2013

Contributor

Hi devdazed, thanks for responding so quickly. Can you think of another way to identify the value being passed in is a UUID? I know many DB providers use the data type of the value to determine the type of encoding or mapping to the underlying DB data type. Thanks

Contributor

azweb76 commented Oct 15, 2013

Hi devdazed, thanks for responding so quickly. Can you think of another way to identify the value being passed in is a UUID? I know many DB providers use the data type of the value to determine the type of encoding or mapping to the underlying DB data type. Thanks

@azweb76

This comment has been minimized.

Show comment Hide comment
@azweb76

azweb76 Oct 28, 2013

Contributor

Any word on this? We are currently passing in text that looks like a UUID, and the query is failing.

Contributor

azweb76 commented Oct 28, 2013

Any word on this? We are currently passing in text that looks like a UUID, and the query is failing.

@asilvas

This comment has been minimized.

Show comment Hide comment
@asilvas

asilvas Oct 28, 2013

@devdazed This "feature" is a bug imo. Auto-converting strings that happen to contain a UUID to a UUID is rather concerning as it is against our will. Keys can be anything, and I'd rather avoid having our data type changed from what we're storing it as. Effectively we're saying "this is our schema", but the helenus does "but we'll auto-convert these for you!".

asilvas commented Oct 28, 2013

@devdazed This "feature" is a bug imo. Auto-converting strings that happen to contain a UUID to a UUID is rather concerning as it is against our will. Keys can be anything, and I'd rather avoid having our data type changed from what we're storing it as. Effectively we're saying "this is our schema", but the helenus does "but we'll auto-convert these for you!".

devdazed added a commit that referenced this pull request Oct 28, 2013

Merge pull request #122 from azweb76/master
Original code assumed that all string objects in the format of a UUID we...

@devdazed devdazed merged commit cbc07d9 into simplereach:master Oct 28, 2013

1 check passed

default The Travis CI build passed
Details
@devdazed

This comment has been minimized.

Show comment Hide comment
@devdazed

devdazed Oct 28, 2013

Member

Sounds good to me. I'll bump & release.

Member

devdazed commented Oct 28, 2013

Sounds good to me. I'll bump & release.

@asilvas

This comment has been minimized.

Show comment Hide comment
@asilvas

asilvas Oct 28, 2013

+1

asilvas commented Oct 28, 2013

+1

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