Skip to content

spanner: Update spansql statement parser version - #1398

Open
adamhicks wants to merge 2 commits into
golang-migrate:masterfrom
adamhicks:adam-updatespansql
Open

spanner: Update spansql statement parser version#1398
adamhicks wants to merge 2 commits into
golang-migrate:masterfrom
adamhicks:adam-updatespansql

Conversation

@adamhicks

@adamhicks adamhicks commented May 18, 2026

Copy link
Copy Markdown

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

Copilot AI review requested due to automatic review settings May 18, 2026 08:36
@adamhicks adamhicks changed the title Adam updatespansql spanner: Update spansql statement parser version May 18, 2026
@adamhicks

Copy link
Copy Markdown
Author

To get these changes I've run:

go get -u cloud.google.com/go/spanner@585840ff8168325dea801e13c398055692c0fe32
go mod tidy

and added a unit test that demonstrates the issue (copy just this test to master to see the issue)

@adamhicks

Copy link
Copy Markdown
Author

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.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 TestCleanStatements sub-test verifying that a uuid column type round-trips as UUID without backtick corruption.
  • Move the supported Go versions to 1.25/1.26 in CI and the README badge, and set go 1.25.0 in go.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.

Comment thread go.mod Outdated
Comment thread database/spanner/spanner_test.go
@coveralls

coveralls commented May 18, 2026

Copy link
Copy Markdown

Coverage Status

coverage: 54.412%. remained the same — adamhicks:adam-updatespansql into golang-migrate:master

@Fontinalis
Fontinalis self-requested a review July 4, 2026 22:32
@Fontinalis

Copy link
Copy Markdown
Member

Hey @adamhicks, thank you for putting the work into this and opening the PR!

Would you mind rebasing this against the latest master? Could you also extend the PR description with a short changes summary, including the Go compatibility change/drop of Go 1.24 support?

Otherwise this looks great on first pass. Thanks again for putting the effort into this!

@Fontinalis Fontinalis added this to the v4.20.0 milestone Jul 4, 2026
@adamhicks
adamhicks force-pushed the adam-updatespansql branch from e25c805 to db796cc Compare July 6, 2026 10:11
@adamhicks

Copy link
Copy Markdown
Author

To get these changes I've run:

go get -u cloud.google.com/go/spanner@585840ff8168325dea801e13c398055692c0fe32
go mod tidy

and added a unit test that demonstrates the issue (copy just this test to master to see the issue)

go/spanner was released so ran

go get cloud.google.com/go/spanner@v1.92.0
go mod tidy

instead (-u not needed)

@adamhicks

Copy link
Copy Markdown
Author

Hey @adamhicks, thank you for putting the work into this and opening the PR!

Would you mind rebasing this against the latest master? Could you also extend the PR description with a short changes summary, including the Go compatibility change/drop of Go 1.24 support?

Otherwise this looks great on first pass. Thanks again for putting the effort into this!

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 1.25.8, i've included this in the description as well. Finally, the go.sum had a bunch of old revision hashes which have been tidied up as well.

@adamhicks

adamhicks commented Jul 10, 2026

Copy link
Copy Markdown
Author

@Fontinalis is this good to go? 🙏

@adamhicks

Copy link
Copy Markdown
Author

Sorry to be a pain, we could really use this being in the next release 🙏
@Fontinalis

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants