-
Notifications
You must be signed in to change notification settings - Fork 562
Description
In a specific situation, eager loading and relation methods panic. They are separate issues, but occurs when using the same schema, and related to each other, so I opened this as a single issue.
If we using MySQL and having schema like this:
CREATE TABLE `category` (
`id` BIGINT UNSIGNED NOT NULL,
PRIMARY KEY (`id`)
);
CREATE TABLE `article` (
`id` BIGINT UNSIGNED NOT NULL,
`category_id` BIGINT UNSIGNED DEFAULT NULL,
PRIMARY KEY (`id`),
FOREIGN KEY (`category_id`) REFERENCES `category` (`id`)
);and following data:
mysql> select * from article;
+----+---------------------+
| id | category_id |
+----+---------------------+
| 1 | 9223372036854775808 |
+----+---------------------+
mysql> select * from category;
+---------------------+
| id |
+---------------------+
| 9223372036854775808 |
+---------------------+
then eager loading panic:
articlesWithCategory, _ := models.
Articles(qm.Load(models.ArticleRels.Category)).
All(ctx, db)
// panic: primitive type of a (string) was not the same primitive type as b (int64)Also, relation method panic:
article, _ := models.
Articles(models.ArticleWhere.ID.EQ(1)).
One(ctx, db)
_ := article.SetCategory(ctx, db, false, category)
// panic: tried to call Scan on *null.Uint64 with <nil> but got err: converting driver.Value type int64 ("-9223372036854775808") to a uint64: invalid syntaxThis issue occurs only if
- The column with a foreign key (
article.category_id) isBIGINT UNSIGNED DEFAULT NULL - The referenced table's id (
category.id) isBIGINT UNSIGNED NOT NULL - We have large
uint64for referenced table's id, which overflows the range ofint64
Details
For eager loading, the panic is from queries.Equal().
- Assume
queries.Equal()called withnull.Uint64(a) anduint64(b) - a is
sql.Valuer, so thata.Value()called, then a is converted tostringnull.Uint64.Value()returns string when the value overflows range of int64
- b is converted to
int64 - reflect.Type of
stringandint64is not the same panic()called
For relation methods, the panic is from queries.Assign().
- Assume
queries.Assign()is called withnull.Uint64(dst) anduint64(src) - dst is
sql.Scanner, so thatdst.Scan(src)is called null.Uint64.Scan()internally usesstrconv.ParseUint(), which returns an error when the argument has minus sign- An error returned from
null.Uint64.Scan() panic()called
Possible solution
For eager loading, fixing queries.Equal() will resolve the issue.
- If one of a or b is a string and another is a number (after acquiring actual value form
driver.Valuer), format number to string before comparison. - I have an intention to submit a PR this way.
This change will introduce some performance overhead, but comparing in such a way is relatively rare to happen.
The behavior of null.Uint64.Value is a little tricky in the first place, but I think it is ok to return large uint64 as a formatted string from the Value() method, because Value() should return driver.Value which doesn't include uint64. (cf. https://golang.org/pkg/database/sql/driver/#Value)
For relation methods, fixing null.Uint64.Scan() will resolve the issue.
- If the argument is
int64and has a minus sign, then convert it touint64, then pass tostrconv.ParseUint() - I also have an intention to submit a PR (to the null package's repository).
This is better than fixing queries.Assign() to pass uint64 to null.Uint64.Scan(), because sql.Scanner.Scan() can't receive uint64 basically. (cf. https://golang.org/pkg/database/sql/#Scanner)