spanner: Update spansql statement parser version#1398
Conversation
adamhicks
changed the title
|
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.
|
|
||
| require ( | ||
| cloud.google.com/go/spanner v1.85.0 | ||
| cloud.google.com/go/spanner v1.91.1-0.20260518075127-585840ff8168 |
There was a problem hiding this comment.
I understand if this is a blocking concern, but not sure when google will release the relevant change
| name: "can clean uuid column types", | ||
| multiStatement: "CREATE TABLE table_name (id uuid NOT NULL) PRIMARY KEY (id);", | ||
| expected: []string{"CREATE TABLE table_name (\n id UUID NOT NULL,\n) PRIMARY KEY(id)"}, | ||
| }, | ||
| { |
|
coverage: 54.432%. remained the same — adamhicks:adam-updatespansql into golang-migrate:master |
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.