spanner: Update spansql statement parser version - #1398
Conversation
|
To get these changes I've run: and added a unit test that demonstrates the issue (copy just this test to |
|
Updating spanner required go 1.25 so I've also upgraded the repo to support 1.25/1.26 and drop 1.24. I'm aware that this may be controversial, so I'm happy to discuss and we can make this change separately if preferred. |
There was a problem hiding this comment.
Pull request overview
Pulls in an upstream cloud.google.com/go/spanner fix that prevented spansql from corrupting UUID column types (it was wrapping the type in backticks). The bump is accompanied by a regression test in database/spanner/spanner_test.go and refreshed transitive Go dependencies, plus a bump of the supported Go toolchain matrix from 1.24/1.25 to 1.25/1.26.
Changes:
- Bump
cloud.google.com/go/spanner(to a pseudo-version that includes the UUID fix) and update many transitive deps. - Add a
TestCleanStatementssub-test verifying that auuidcolumn type round-trips asUUIDwithout backtick corruption. - Move the supported Go versions to
1.25/1.26in CI and the README badge, and setgo 1.25.0ingo.mod.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| go.mod | Bumps spanner (to an unreleased pseudo-version) and refreshes many indirect dependencies; bumps go directive to 1.25.0. |
| database/spanner/spanner_test.go | Adds a cleanStatements test case asserting uuid is normalized to UUID without backticks. |
| .github/workflows/ci.yaml | Updates the CI Go matrix from 1.24.x/1.25.x to 1.25.x/1.26.x. |
| README.md | Updates the "Supported Go Versions" badge to 1.25, 1.26. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Hey @adamhicks, thank you for putting the work into this and opening the PR! Would you mind rebasing this against the latest Otherwise this looks great on first pass. Thanks again for putting the effort into this! |
e25c805 to
db796cc
Compare
go/spanner was released so ran instead (-u not needed) |
Thanks, I've updated the description with all the changes to the spansql package between 1.85.0 and 1.92.0 (only four commits). With the rebase, it looks like support for go has already moved ahead to 1.25 so i've only needed to update the patch version to |
|
@Fontinalis is this good to go? 🙏 |
|
Sorry to be a pain, we could really use this being in the next release 🙏 |
I recently spotted an issue with spansql to do with UUID column types. When golang-migrate was cleaning the statements, it actually corrupted the uuid columns by adding backticks '`' to the data type.
I raised this issue and a PR to fix it.
The PR has been merged so now I'm looking to get this change merged into golang-migrate.
Changes
go/spannerfrom 1.85.0 -> 1.92.0:spansqlin this upgradeSTRUCTtypes https://docs.cloud.google.com/spanner/docs/structs1.25.0to1.25.11go mod tidyto remove a bunch of old package hashes fromgo.sum