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.

Sign up for GitHub

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

Jump to bottom

fix: Spanner hash function returns string value instead of bytes #1062

Merged
merged 1 commit into from
Nov 30, 2023
Merged

fix: Spanner hash function returns string value instead of bytes #1062

merged 1 commit into from
Nov 30, 2023

Conversation

nehanene15
Copy link
Collaborator

Closes #1060

  • Fixes the old incorrect hash function sha2() and replaces with the correct function TO_HEX(SHA256(t4.concat__all)) which returns a string value that can be compared to BQ and other DB's hash output
  • Adds tests for row validation with BQ
  • Removes using the third_party.ibis.ibis_spanner.conftest fixtures since it was spinning up new Spanner instances for each test. Instead, we reference a single, stateful Spanner instance in our test project similar to our other DBs.

@nehanene15 nehanene15 requested a review from a team as a code owner November 30, 2023 01:35
@nehanene15
Copy link
Collaborator Author

/gcbrun

sundar-mudupalli-work
sundar-mudupalli-work reviewed Nov 30, 2023
View reviewed changes
tests/system/data_sources/test_spanner.py
Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM. Thanks.

sundar-mudupalli-work
sundar-mudupalli-work reviewed Nov 30, 2023
View reviewed changes
third_party/ibis/ibis_addon/operations.py
Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM.

Thanks.

sundar-mudupalli-work
sundar-mudupalli-work approved these changes Nov 30, 2023
View reviewed changes
Copy link
Contributor

@sundar-mudupalli-work sundar-mudupalli-work left a comment

Choose a reason for hiding this comment

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

Neha,

I checked this change and it resolved #1060.

Thank you.

Sundar Mudupalli

@nehanene15 nehanene15 merged commit 722dff9 into develop Nov 30, 2023
5 checks passed
@nehanene15 nehanene15 deleted the issue1060-spanner branch November 30, 2023 14:37
@release-please release-please bot mentioned this pull request Nov 30, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sundar-mudupalli-work sundar-mudupalli-work sundar-mudupalli-work approved these changes

@helensilva14 helensilva14 Awaiting requested review from helensilva14

Assignees
No one assigned
Labels
size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

MySQL to Spanner comparison fails for text fields
2 participants
@nehanene15 @sundar-mudupalli-work