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.

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

migrate up fails on negative int #213

Closed
brondum opened this issue Apr 29, 2019 · 5 comments
Closed

migrate up fails on negative int #213

brondum opened this issue Apr 29, 2019 · 5 comments

Comments

@brondum
Copy link

brondum commented Apr 29, 2019

Describe the Bug
Getting a weird error when trying to migrate initial db scheme on ARM.
i dont see this error on amd64, and as far is i know the number is "large" enough that 32bit limit is hit?

Am i missing something?

Expected Behavior
Migration as when running on x86_64 platform

Migrate Version
v4.3.1

Loaded Source Drivers
file

Loaded Database Drivers
mysql

Go Version
go version go1.12 linux/arm

Stacktrace

panic: suint(-941147960) expects input >= 0

goroutine 26 [running]:
github.com/golang-migrate/migrate/v4.suint(...)
	/go/src/gitlab.com/projectname/vendor/github.com/golang-migrate/migrate/v4/util.go:42
github.com/golang-migrate/migrate/v4.(*Migrate).readUp(0x20f1a40, 0xffffffff, 0xffffffff, 0x20f1c40)
	/go/src/gitlab.com/projectname/vendor/github.com/golang-migrate/migrate/v4/migrate.go:580 +0x688
created by github.com/golang-migrate/migrate/v4.(*Migrate).Up
	/go/src/gitlab.com/projectname/vendor/github.com/golang-migrate/migrate/v4/migrate.go:281 +0xf0

Additional context
I am only using signed INTs in the db scheme.

@dhui
Copy link
Member

dhui commented Apr 29, 2019

What do your migration sources look like? e.g. how many migrations are there?

The value -941147960 doesn't stick out to me...
e.g. It's not obvious to me how that value is the result of a 32-bit int overflow

@brondum
Copy link
Author

brondum commented Apr 30, 2019

@dhui Thank you for the quick reply.

only have one migration:

20181110175944_initialize.down.sql
20181110175944_initialize.up.sql

@dhui
Copy link
Member

dhui commented Apr 30, 2019

The issue is that the date is being used as the migration version number, which overflow 32-bit ints
I've reproduced the issue here: https://play.golang.org/p/T3OQbMm08Ix

package main

import (
	"fmt"
	"strconv"
)

func main() {
	s := "20181110175944"
	i, err := strconv.ParseUint(s, 10, 64)
	if err != nil {
		fmt.Println("Got parsing error:", err)
		return
	}
	fmt.Println("Parsed int:", i)
	ui32 := uint32(i)
	fmt.Println("Uint32:", ui32)
	i32 := int32(i)
	fmt.Println("Int32:", i32)
}
/* Prints:
Parsed int: 20181110175944
Uint32: 3353819336
Int32: -941147960
*/

For now, the workaround is to specify either the -format or -seq option when running migrate create to avoid the overflow issue. Personally, I prefer to use -seq since the version numbers are more readable.
See migrate create -h for more info.

To fix this issue in the longterm, we could explicitly use uint64 (or our own custom type) as the version number. Such a change would result in a change in the interface, breaking backwards compatibility.

@dhui
Copy link
Member

dhui commented Apr 30, 2019

I've create a separate issue to track the longterm fix: #214

@brondum
Copy link
Author

brondum commented May 1, 2019

@dhui
Thank you so much for the help and the investigation on how to reproduce it. That was dope ;)
I will close this and follow the other issue.

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

No branches or pull requests

2 participants