From 4162cfd0a3d7277115e7e5beff5e4bbf3ef13741 Mon Sep 17 00:00:00 2001 From: Dmytro Solovei Date: Wed, 28 Dec 2022 12:27:55 +0100 Subject: [PATCH 1/7] test: illustrate the bug The snapshot diff will show difference in case (got VARCHAR(255), want varchar(10)). That's not the bug and only done because CreateTableQuery.appendSQLType currently appends lowercase "varchar". --- internal/dbtest/query_test.go | 13 +++++++++++++ .../dbtest/testdata/snapshots/TestQuery-mariadb-156 | 1 + .../testdata/snapshots/TestQuery-mssql2019-156 | 1 + .../dbtest/testdata/snapshots/TestQuery-mysql5-156 | 1 + .../dbtest/testdata/snapshots/TestQuery-mysql8-156 | 1 + internal/dbtest/testdata/snapshots/TestQuery-pg-156 | 1 + .../dbtest/testdata/snapshots/TestQuery-pgx-156 | 1 + .../dbtest/testdata/snapshots/TestQuery-sqlite-156 | 1 + 8 files changed, 20 insertions(+) create mode 100644 internal/dbtest/testdata/snapshots/TestQuery-mariadb-156 create mode 100644 internal/dbtest/testdata/snapshots/TestQuery-mssql2019-156 create mode 100644 internal/dbtest/testdata/snapshots/TestQuery-mysql5-156 create mode 100644 internal/dbtest/testdata/snapshots/TestQuery-mysql8-156 create mode 100644 internal/dbtest/testdata/snapshots/TestQuery-pg-156 create mode 100644 internal/dbtest/testdata/snapshots/TestQuery-pgx-156 create mode 100644 internal/dbtest/testdata/snapshots/TestQuery-sqlite-156 diff --git a/internal/dbtest/query_test.go b/internal/dbtest/query_test.go index 0c8448568..c8dca0144 100644 --- a/internal/dbtest/query_test.go +++ b/internal/dbtest/query_test.go @@ -969,6 +969,19 @@ func TestQuery(t *testing.T) { When("NOT MATCHED THEN INSERT (name, value) VALUES (_data.name, _data.value)"). Returning("$action") }, + func(db *bun.DB) schema.QueryAppender { + // Note: not all dialects require specifying VARCHAR length + type Model struct { + // ID has the reflection-based type (DiscoveredSQLType) with default length + ID string + // Name has specific type and length defined (UserSQLType) + Name string `bun:",type:varchar(50)"` + // Title has user-defined type (UserSQLType) with default length + Title string `bun:",type:varchar"` + } + // Set default VARCHAR length to 10 + return db.NewCreateTable().Model((*Model)(nil)).Varchar(10) + }, } timeRE := regexp.MustCompile(`'2\d{3}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}(\.\d+)?(\+\d{2}:\d{2})?'`) diff --git a/internal/dbtest/testdata/snapshots/TestQuery-mariadb-156 b/internal/dbtest/testdata/snapshots/TestQuery-mariadb-156 new file mode 100644 index 000000000..9c07af84e --- /dev/null +++ b/internal/dbtest/testdata/snapshots/TestQuery-mariadb-156 @@ -0,0 +1 @@ +CREATE TABLE `models` (`id` varchar(10), `name` varchar(50), `title` varchar(10)) diff --git a/internal/dbtest/testdata/snapshots/TestQuery-mssql2019-156 b/internal/dbtest/testdata/snapshots/TestQuery-mssql2019-156 new file mode 100644 index 000000000..453f6ca9f --- /dev/null +++ b/internal/dbtest/testdata/snapshots/TestQuery-mssql2019-156 @@ -0,0 +1 @@ +CREATE TABLE "models" ("id" varchar(10), "name" varchar(50), "title" varchar(10)) diff --git a/internal/dbtest/testdata/snapshots/TestQuery-mysql5-156 b/internal/dbtest/testdata/snapshots/TestQuery-mysql5-156 new file mode 100644 index 000000000..9c07af84e --- /dev/null +++ b/internal/dbtest/testdata/snapshots/TestQuery-mysql5-156 @@ -0,0 +1 @@ +CREATE TABLE `models` (`id` varchar(10), `name` varchar(50), `title` varchar(10)) diff --git a/internal/dbtest/testdata/snapshots/TestQuery-mysql8-156 b/internal/dbtest/testdata/snapshots/TestQuery-mysql8-156 new file mode 100644 index 000000000..9c07af84e --- /dev/null +++ b/internal/dbtest/testdata/snapshots/TestQuery-mysql8-156 @@ -0,0 +1 @@ +CREATE TABLE `models` (`id` varchar(10), `name` varchar(50), `title` varchar(10)) diff --git a/internal/dbtest/testdata/snapshots/TestQuery-pg-156 b/internal/dbtest/testdata/snapshots/TestQuery-pg-156 new file mode 100644 index 000000000..453f6ca9f --- /dev/null +++ b/internal/dbtest/testdata/snapshots/TestQuery-pg-156 @@ -0,0 +1 @@ +CREATE TABLE "models" ("id" varchar(10), "name" varchar(50), "title" varchar(10)) diff --git a/internal/dbtest/testdata/snapshots/TestQuery-pgx-156 b/internal/dbtest/testdata/snapshots/TestQuery-pgx-156 new file mode 100644 index 000000000..453f6ca9f --- /dev/null +++ b/internal/dbtest/testdata/snapshots/TestQuery-pgx-156 @@ -0,0 +1 @@ +CREATE TABLE "models" ("id" varchar(10), "name" varchar(50), "title" varchar(10)) diff --git a/internal/dbtest/testdata/snapshots/TestQuery-sqlite-156 b/internal/dbtest/testdata/snapshots/TestQuery-sqlite-156 new file mode 100644 index 000000000..453f6ca9f --- /dev/null +++ b/internal/dbtest/testdata/snapshots/TestQuery-sqlite-156 @@ -0,0 +1 @@ +CREATE TABLE "models" ("id" varchar(10), "name" varchar(50), "title" varchar(10)) From e5079c70343ba8c8b410aed23ac1d1ae5a2c9ff6 Mon Sep 17 00:00:00 2001 From: Dmytro Solovei Date: Wed, 28 Dec 2022 13:45:17 +0100 Subject: [PATCH 2/7] fix: append default VARCHAR length instead of hardcoding it in the type definition + use strings.EqualFold() for case-insensitive string comparison (SQL is case-insensitive) + Dialect interface now requires `DefaultVarcharLen()` + CreateTableQuery uses the length set in .Varchar() also for `bun:",type:varchar"` --- dialect/mssqldialect/dialect.go | 6 ++++-- dialect/mysqldialect/dialect.go | 9 +++++---- dialect/pgdialect/sqltype.go | 4 ++++ dialect/sqlitedialect/dialect.go | 4 ++++ query_table_create.go | 17 +++++++++++++---- schema/dialect.go | 13 +++++++++++-- 6 files changed, 41 insertions(+), 12 deletions(-) diff --git a/dialect/mssqldialect/dialect.go b/dialect/mssqldialect/dialect.go index e14824b8f..a56eb0c29 100755 --- a/dialect/mssqldialect/dialect.go +++ b/dialect/mssqldialect/dialect.go @@ -127,10 +127,12 @@ func (*Dialect) AppendBool(b []byte, v bool) []byte { return strconv.AppendUint(b, uint64(num), 10) } +func (d *Dialect) DefaultVarcharLen() int { + return 255 +} + func sqlType(field *schema.Field) string { switch field.DiscoveredSQLType { - case sqltype.VarChar: - return field.DiscoveredSQLType + "(255)" case sqltype.Timestamp: return datetimeType case sqltype.Boolean: diff --git a/dialect/mysqldialect/dialect.go b/dialect/mysqldialect/dialect.go index 4b16b4a22..9e9032e2c 100644 --- a/dialect/mysqldialect/dialect.go +++ b/dialect/mysqldialect/dialect.go @@ -172,11 +172,12 @@ func (*Dialect) AppendJSON(b, jsonb []byte) []byte { return b } +func (d *Dialect) DefaultVarcharLen() int { + return 255 +} + func sqlType(field *schema.Field) string { - switch field.DiscoveredSQLType { - case sqltype.VarChar: - return field.DiscoveredSQLType + "(255)" - case sqltype.Timestamp: + if field.DiscoveredSQLType == sqltype.Timestamp { return datetimeType } return field.DiscoveredSQLType diff --git a/dialect/pgdialect/sqltype.go b/dialect/pgdialect/sqltype.go index 6c6294d71..dadea5c1c 100644 --- a/dialect/pgdialect/sqltype.go +++ b/dialect/pgdialect/sqltype.go @@ -45,6 +45,10 @@ var ( jsonRawMessageType = reflect.TypeOf((*json.RawMessage)(nil)).Elem() ) +func (d *Dialect) DefaultVarcharLen() int { + return 0 +} + func fieldSQLType(field *schema.Field) string { if field.UserSQLType != "" { return field.UserSQLType diff --git a/dialect/sqlitedialect/dialect.go b/dialect/sqlitedialect/dialect.go index 720e979f5..3c809e7a7 100644 --- a/dialect/sqlitedialect/dialect.go +++ b/dialect/sqlitedialect/dialect.go @@ -87,6 +87,10 @@ func (d *Dialect) AppendBytes(b []byte, bs []byte) []byte { return b } +func (d *Dialect) DefaultVarcharLen() int { + return 0 +} + func fieldSQLType(field *schema.Field) string { switch field.DiscoveredSQLType { case sqltype.SmallInt, sqltype.BigInt: diff --git a/query_table_create.go b/query_table_create.go index 002250bc1..e0629dde0 100644 --- a/query_table_create.go +++ b/query_table_create.go @@ -5,6 +5,7 @@ import ( "database/sql" "sort" "strconv" + "strings" "github.com/uptrace/bun/dialect/feature" "github.com/uptrace/bun/dialect/sqltype" @@ -32,6 +33,7 @@ func NewCreateTableQuery(db *DB) *CreateTableQuery { db: db, conn: db.DB, }, + varchar: db.Dialect().DefaultVarcharLen(), } return q } @@ -82,6 +84,10 @@ func (q *CreateTableQuery) IfNotExists() *CreateTableQuery { return q } +// Varchar changes the default length for VARCHAR columns. +// Because some dialects require that length is always specified for VARCHAR type, +// we will use the exact user-defined type if length is set explicitly, as in `bun:",type:varchar(5)"`, +// but assume the new default length when it's omitted, e.g. `bun:",type:varchar"`. func (q *CreateTableQuery) Varchar(n int) *CreateTableQuery { q.varchar = n return q @@ -120,7 +126,7 @@ func (q *CreateTableQuery) WithForeignKeys() *CreateTableQuery { return q } -//------------------------------------------------------------------------------ +// ------------------------------------------------------------------------------ func (q *CreateTableQuery) Operation() string { return "CREATE TABLE" @@ -221,12 +227,15 @@ func (q *CreateTableQuery) AppendQuery(fmter schema.Formatter, b []byte) (_ []by } func (q *CreateTableQuery) appendSQLType(b []byte, field *schema.Field) []byte { - if field.CreateTableSQLType != field.DiscoveredSQLType { + // Most of the time these two will match, but for the cases where DiscoveredSQLType is dialect-specific, + // e.g. pgdialect would change sqltype.SmallInt to pgTypeSmallSerial for columns that have `bun:",autoincrement"` + if !strings.EqualFold(field.CreateTableSQLType, field.DiscoveredSQLType) { return append(b, field.CreateTableSQLType...) } - if q.varchar > 0 && - field.CreateTableSQLType == sqltype.VarChar { + // For all common SQL types except VARCHAR, both UserDefinedSQLType and DiscoveredSQLType specify the correct type, + // and we needn't modify it. For VARCHAR columns, we will stop to check if a valid length has been set in .Varchar(int). + if q.varchar > 0 && strings.EqualFold(field.CreateTableSQLType, sqltype.VarChar) { b = append(b, "varchar("...) b = strconv.AppendInt(b, int64(q.varchar), 10) b = append(b, ")"...) diff --git a/schema/dialect.go b/schema/dialect.go index b73d89bd0..fea8238dc 100644 --- a/schema/dialect.go +++ b/schema/dialect.go @@ -30,9 +30,14 @@ type Dialect interface { AppendBytes(b []byte, bs []byte) []byte AppendJSON(b, jsonb []byte) []byte AppendBool(b []byte, v bool) []byte + + // DefaultVarcharLen should be returned for dialects in which specifying VARCHAR length + // is mandatory in queries that modify the schema (CREATE TABLE / ADD COLUMN, etc). + // Dialects that do not have such requirement may return 0, which should be interpreted so by the caller. + DefaultVarcharLen() int } -//------------------------------------------------------------------------------ +// ------------------------------------------------------------------------------ type BaseDialect struct{} @@ -131,7 +136,7 @@ func (BaseDialect) AppendBool(b []byte, v bool) []byte { return dialect.AppendBool(b, v) } -//------------------------------------------------------------------------------ +// ------------------------------------------------------------------------------ type nopDialect struct { BaseDialect @@ -168,3 +173,7 @@ func (d *nopDialect) OnTable(table *Table) {} func (d *nopDialect) IdentQuote() byte { return '"' } + +func (d *nopDialect) DefaultVarcharLen() int { + return 0 +} From dc2768c1e2a08ed04108e8aa05bcead70b82b855 Mon Sep 17 00:00:00 2001 From: Dmytro Solovei Date: Wed, 28 Dec 2022 14:03:07 +0100 Subject: [PATCH 3/7] refactor: append sqltype.VarChar instead of the hardcoded "varchar" + Return early to improve readability --- .../dbtest/testdata/snapshots/TestQuery-mariadb-156 | 2 +- .../testdata/snapshots/TestQuery-mssql2019-156 | 2 +- .../dbtest/testdata/snapshots/TestQuery-mysql5-156 | 2 +- .../dbtest/testdata/snapshots/TestQuery-mysql8-156 | 2 +- internal/dbtest/testdata/snapshots/TestQuery-pg-156 | 2 +- .../dbtest/testdata/snapshots/TestQuery-pgx-156 | 2 +- .../dbtest/testdata/snapshots/TestQuery-sqlite-156 | 2 +- query_table_create.go | 13 +++++++------ 8 files changed, 14 insertions(+), 13 deletions(-) diff --git a/internal/dbtest/testdata/snapshots/TestQuery-mariadb-156 b/internal/dbtest/testdata/snapshots/TestQuery-mariadb-156 index 9c07af84e..117474bd2 100644 --- a/internal/dbtest/testdata/snapshots/TestQuery-mariadb-156 +++ b/internal/dbtest/testdata/snapshots/TestQuery-mariadb-156 @@ -1 +1 @@ -CREATE TABLE `models` (`id` varchar(10), `name` varchar(50), `title` varchar(10)) +CREATE TABLE `models` (`id` VARCHAR(10), `name` varchar(50), `title` VARCHAR(10)) diff --git a/internal/dbtest/testdata/snapshots/TestQuery-mssql2019-156 b/internal/dbtest/testdata/snapshots/TestQuery-mssql2019-156 index 453f6ca9f..04ad92c73 100644 --- a/internal/dbtest/testdata/snapshots/TestQuery-mssql2019-156 +++ b/internal/dbtest/testdata/snapshots/TestQuery-mssql2019-156 @@ -1 +1 @@ -CREATE TABLE "models" ("id" varchar(10), "name" varchar(50), "title" varchar(10)) +CREATE TABLE "models" ("id" VARCHAR(10), "name" varchar(50), "title" VARCHAR(10)) diff --git a/internal/dbtest/testdata/snapshots/TestQuery-mysql5-156 b/internal/dbtest/testdata/snapshots/TestQuery-mysql5-156 index 9c07af84e..117474bd2 100644 --- a/internal/dbtest/testdata/snapshots/TestQuery-mysql5-156 +++ b/internal/dbtest/testdata/snapshots/TestQuery-mysql5-156 @@ -1 +1 @@ -CREATE TABLE `models` (`id` varchar(10), `name` varchar(50), `title` varchar(10)) +CREATE TABLE `models` (`id` VARCHAR(10), `name` varchar(50), `title` VARCHAR(10)) diff --git a/internal/dbtest/testdata/snapshots/TestQuery-mysql8-156 b/internal/dbtest/testdata/snapshots/TestQuery-mysql8-156 index 9c07af84e..117474bd2 100644 --- a/internal/dbtest/testdata/snapshots/TestQuery-mysql8-156 +++ b/internal/dbtest/testdata/snapshots/TestQuery-mysql8-156 @@ -1 +1 @@ -CREATE TABLE `models` (`id` varchar(10), `name` varchar(50), `title` varchar(10)) +CREATE TABLE `models` (`id` VARCHAR(10), `name` varchar(50), `title` VARCHAR(10)) diff --git a/internal/dbtest/testdata/snapshots/TestQuery-pg-156 b/internal/dbtest/testdata/snapshots/TestQuery-pg-156 index 453f6ca9f..04ad92c73 100644 --- a/internal/dbtest/testdata/snapshots/TestQuery-pg-156 +++ b/internal/dbtest/testdata/snapshots/TestQuery-pg-156 @@ -1 +1 @@ -CREATE TABLE "models" ("id" varchar(10), "name" varchar(50), "title" varchar(10)) +CREATE TABLE "models" ("id" VARCHAR(10), "name" varchar(50), "title" VARCHAR(10)) diff --git a/internal/dbtest/testdata/snapshots/TestQuery-pgx-156 b/internal/dbtest/testdata/snapshots/TestQuery-pgx-156 index 453f6ca9f..04ad92c73 100644 --- a/internal/dbtest/testdata/snapshots/TestQuery-pgx-156 +++ b/internal/dbtest/testdata/snapshots/TestQuery-pgx-156 @@ -1 +1 @@ -CREATE TABLE "models" ("id" varchar(10), "name" varchar(50), "title" varchar(10)) +CREATE TABLE "models" ("id" VARCHAR(10), "name" varchar(50), "title" VARCHAR(10)) diff --git a/internal/dbtest/testdata/snapshots/TestQuery-sqlite-156 b/internal/dbtest/testdata/snapshots/TestQuery-sqlite-156 index 453f6ca9f..04ad92c73 100644 --- a/internal/dbtest/testdata/snapshots/TestQuery-sqlite-156 +++ b/internal/dbtest/testdata/snapshots/TestQuery-sqlite-156 @@ -1 +1 @@ -CREATE TABLE "models" ("id" varchar(10), "name" varchar(50), "title" varchar(10)) +CREATE TABLE "models" ("id" VARCHAR(10), "name" varchar(50), "title" VARCHAR(10)) diff --git a/query_table_create.go b/query_table_create.go index e0629dde0..f78889560 100644 --- a/query_table_create.go +++ b/query_table_create.go @@ -235,14 +235,15 @@ func (q *CreateTableQuery) appendSQLType(b []byte, field *schema.Field) []byte { // For all common SQL types except VARCHAR, both UserDefinedSQLType and DiscoveredSQLType specify the correct type, // and we needn't modify it. For VARCHAR columns, we will stop to check if a valid length has been set in .Varchar(int). - if q.varchar > 0 && strings.EqualFold(field.CreateTableSQLType, sqltype.VarChar) { - b = append(b, "varchar("...) - b = strconv.AppendInt(b, int64(q.varchar), 10) - b = append(b, ")"...) - return b + if !strings.EqualFold(field.CreateTableSQLType, sqltype.VarChar) || q.varchar <= 0 { + return append(b, field.CreateTableSQLType...) } - return append(b, field.CreateTableSQLType...) + b = append(b, sqltype.VarChar...) + b = append(b, "("...) + b = strconv.AppendInt(b, int64(q.varchar), 10) + b = append(b, ")"...) + return b } func (q *CreateTableQuery) appendUniqueConstraints(fmter schema.Formatter, b []byte) []byte { From 6003beb3a977221a27b9eaf1fb473efa1c835892 Mon Sep 17 00:00:00 2001 From: Dmytro Solovei Date: Wed, 28 Dec 2022 15:23:56 +0100 Subject: [PATCH 4/7] feat(BREAKING): accept only positive values for VARCHAR length --- dialect/mssqldialect/dialect.go | 2 +- dialect/mysqldialect/dialect.go | 2 +- dialect/pgdialect/sqltype.go | 2 +- dialect/sqlitedialect/dialect.go | 2 +- query_table_create.go | 13 +++++++------ schema/dialect.go | 4 ++-- 6 files changed, 13 insertions(+), 12 deletions(-) diff --git a/dialect/mssqldialect/dialect.go b/dialect/mssqldialect/dialect.go index a56eb0c29..a82d99bf7 100755 --- a/dialect/mssqldialect/dialect.go +++ b/dialect/mssqldialect/dialect.go @@ -127,7 +127,7 @@ func (*Dialect) AppendBool(b []byte, v bool) []byte { return strconv.AppendUint(b, uint64(num), 10) } -func (d *Dialect) DefaultVarcharLen() int { +func (d *Dialect) DefaultVarcharLen() uint { return 255 } diff --git a/dialect/mysqldialect/dialect.go b/dialect/mysqldialect/dialect.go index 9e9032e2c..2dc4d325e 100644 --- a/dialect/mysqldialect/dialect.go +++ b/dialect/mysqldialect/dialect.go @@ -172,7 +172,7 @@ func (*Dialect) AppendJSON(b, jsonb []byte) []byte { return b } -func (d *Dialect) DefaultVarcharLen() int { +func (d *Dialect) DefaultVarcharLen() uint { return 255 } diff --git a/dialect/pgdialect/sqltype.go b/dialect/pgdialect/sqltype.go index dadea5c1c..8992b654e 100644 --- a/dialect/pgdialect/sqltype.go +++ b/dialect/pgdialect/sqltype.go @@ -45,7 +45,7 @@ var ( jsonRawMessageType = reflect.TypeOf((*json.RawMessage)(nil)).Elem() ) -func (d *Dialect) DefaultVarcharLen() int { +func (d *Dialect) DefaultVarcharLen() uint { return 0 } diff --git a/dialect/sqlitedialect/dialect.go b/dialect/sqlitedialect/dialect.go index 3c809e7a7..ecd881d6a 100644 --- a/dialect/sqlitedialect/dialect.go +++ b/dialect/sqlitedialect/dialect.go @@ -87,7 +87,7 @@ func (d *Dialect) AppendBytes(b []byte, bs []byte) []byte { return b } -func (d *Dialect) DefaultVarcharLen() int { +func (d *Dialect) DefaultVarcharLen() uint { return 0 } diff --git a/query_table_create.go b/query_table_create.go index f78889560..4347ae3f8 100644 --- a/query_table_create.go +++ b/query_table_create.go @@ -18,7 +18,12 @@ type CreateTableQuery struct { temp bool ifNotExists bool - varchar int + + // varchar changes the default length for VARCHAR columns. + // Because some dialects require that length is always specified for VARCHAR type, + // we will use the exact user-defined type if length is set explicitly, as in `bun:",type:varchar(5)"`, + // but assume the new default length when it's omitted, e.g. `bun:",type:varchar"`. + varchar uint fks []schema.QueryWithArgs partitionBy schema.QueryWithArgs @@ -84,11 +89,7 @@ func (q *CreateTableQuery) IfNotExists() *CreateTableQuery { return q } -// Varchar changes the default length for VARCHAR columns. -// Because some dialects require that length is always specified for VARCHAR type, -// we will use the exact user-defined type if length is set explicitly, as in `bun:",type:varchar(5)"`, -// but assume the new default length when it's omitted, e.g. `bun:",type:varchar"`. -func (q *CreateTableQuery) Varchar(n int) *CreateTableQuery { +func (q *CreateTableQuery) Varchar(n uint) *CreateTableQuery { q.varchar = n return q } diff --git a/schema/dialect.go b/schema/dialect.go index fea8238dc..96de08888 100644 --- a/schema/dialect.go +++ b/schema/dialect.go @@ -34,7 +34,7 @@ type Dialect interface { // DefaultVarcharLen should be returned for dialects in which specifying VARCHAR length // is mandatory in queries that modify the schema (CREATE TABLE / ADD COLUMN, etc). // Dialects that do not have such requirement may return 0, which should be interpreted so by the caller. - DefaultVarcharLen() int + DefaultVarcharLen() uint } // ------------------------------------------------------------------------------ @@ -174,6 +174,6 @@ func (d *nopDialect) IdentQuote() byte { return '"' } -func (d *nopDialect) DefaultVarcharLen() int { +func (d *nopDialect) DefaultVarcharLen() uint { return 0 } From 2c350a360a20ec04f2b20f14a3d824a605c7a9fa Mon Sep 17 00:00:00 2001 From: dyma solovei <53943884+bevzzz@users.noreply.github.com> Date: Wed, 28 Dec 2022 21:02:11 +0100 Subject: [PATCH 5/7] chore: add documentation for `Varchar()` method --- query_table_create.go | 1 + 1 file changed, 1 insertion(+) diff --git a/query_table_create.go b/query_table_create.go index 4347ae3f8..3b8a34a06 100644 --- a/query_table_create.go +++ b/query_table_create.go @@ -89,6 +89,7 @@ func (q *CreateTableQuery) IfNotExists() *CreateTableQuery { return q } +// Varchar sets default length for VARCHAR columns. func (q *CreateTableQuery) Varchar(n uint) *CreateTableQuery { q.varchar = n return q From 36e43ae72e315c2a690bbb9e9b1417ed0f6693fe Mon Sep 17 00:00:00 2001 From: Dmytro Solovei Date: Thu, 29 Dec 2022 11:03:10 +0100 Subject: [PATCH 6/7] revert: "feat(BREAKING): accept only positive values for VARCHAR length" This reverts commit 6003beb3a977221a27b9eaf1fb473efa1c835892. --- dialect/mssqldialect/dialect.go | 2 +- dialect/mysqldialect/dialect.go | 2 +- dialect/pgdialect/sqltype.go | 2 +- dialect/sqlitedialect/dialect.go | 2 +- go.work.sum | 5 +++++ query_table_create.go | 6 +++--- schema/dialect.go | 4 ++-- 7 files changed, 14 insertions(+), 9 deletions(-) diff --git a/dialect/mssqldialect/dialect.go b/dialect/mssqldialect/dialect.go index a82d99bf7..a56eb0c29 100755 --- a/dialect/mssqldialect/dialect.go +++ b/dialect/mssqldialect/dialect.go @@ -127,7 +127,7 @@ func (*Dialect) AppendBool(b []byte, v bool) []byte { return strconv.AppendUint(b, uint64(num), 10) } -func (d *Dialect) DefaultVarcharLen() uint { +func (d *Dialect) DefaultVarcharLen() int { return 255 } diff --git a/dialect/mysqldialect/dialect.go b/dialect/mysqldialect/dialect.go index 2dc4d325e..9e9032e2c 100644 --- a/dialect/mysqldialect/dialect.go +++ b/dialect/mysqldialect/dialect.go @@ -172,7 +172,7 @@ func (*Dialect) AppendJSON(b, jsonb []byte) []byte { return b } -func (d *Dialect) DefaultVarcharLen() uint { +func (d *Dialect) DefaultVarcharLen() int { return 255 } diff --git a/dialect/pgdialect/sqltype.go b/dialect/pgdialect/sqltype.go index 8992b654e..dadea5c1c 100644 --- a/dialect/pgdialect/sqltype.go +++ b/dialect/pgdialect/sqltype.go @@ -45,7 +45,7 @@ var ( jsonRawMessageType = reflect.TypeOf((*json.RawMessage)(nil)).Elem() ) -func (d *Dialect) DefaultVarcharLen() uint { +func (d *Dialect) DefaultVarcharLen() int { return 0 } diff --git a/dialect/sqlitedialect/dialect.go b/dialect/sqlitedialect/dialect.go index ecd881d6a..3c809e7a7 100644 --- a/dialect/sqlitedialect/dialect.go +++ b/dialect/sqlitedialect/dialect.go @@ -87,7 +87,7 @@ func (d *Dialect) AppendBytes(b []byte, bs []byte) []byte { return b } -func (d *Dialect) DefaultVarcharLen() uint { +func (d *Dialect) DefaultVarcharLen() int { return 0 } diff --git a/go.work.sum b/go.work.sum index 05eb5544f..1e0326e1e 100644 --- a/go.work.sum +++ b/go.work.sum @@ -1,7 +1,12 @@ +github.com/cenkalti/backoff v2.2.1+incompatible h1:tNowT99t7UNflLxfYYSlKYsBpXdEet03Pg2g16Swow4= github.com/google/pprof v0.0.0-20221118152302-e6195bd50e26/go.mod h1:dDKJzRmX4S37WGHujM7tX//fmj1uioxKzKxz3lo4HJo= +github.com/grpc-ecosystem/grpc-gateway v1.16.0 h1:gmcG1KaJ57LophUzW0Hy8NmPhnMZb4M0+kPpLofRdBo= +github.com/jackc/chunkreader v1.0.0 h1:4s39bBR8ByfqH+DKm8rQA3E1LHZWB9XWcrz8fqaZbe0= +github.com/jackc/pgproto3 v1.1.0 h1:FYYE4yRw+AgI8wXIinMlNjBbp/UitDJwfj5LqqewP1A= github.com/mattn/go-sqlite3 v1.14.15/go.mod h1:2eHXhiwb8IkHr+BDWZGa96P6+rkvnG63S2DGjv9HUNg= github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= +github.com/urfave/cli v1.22.1 h1:+mkCCcOFKPnCmVYVcURKps1Xe+3zP90gSYGNfRkjoIY= github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= diff --git a/query_table_create.go b/query_table_create.go index 3b8a34a06..22b4e6dee 100644 --- a/query_table_create.go +++ b/query_table_create.go @@ -18,12 +18,12 @@ type CreateTableQuery struct { temp bool ifNotExists bool - + // varchar changes the default length for VARCHAR columns. // Because some dialects require that length is always specified for VARCHAR type, // we will use the exact user-defined type if length is set explicitly, as in `bun:",type:varchar(5)"`, // but assume the new default length when it's omitted, e.g. `bun:",type:varchar"`. - varchar uint + varchar int fks []schema.QueryWithArgs partitionBy schema.QueryWithArgs @@ -90,7 +90,7 @@ func (q *CreateTableQuery) IfNotExists() *CreateTableQuery { } // Varchar sets default length for VARCHAR columns. -func (q *CreateTableQuery) Varchar(n uint) *CreateTableQuery { +func (q *CreateTableQuery) Varchar(n int) *CreateTableQuery { q.varchar = n return q } diff --git a/schema/dialect.go b/schema/dialect.go index 96de08888..fea8238dc 100644 --- a/schema/dialect.go +++ b/schema/dialect.go @@ -34,7 +34,7 @@ type Dialect interface { // DefaultVarcharLen should be returned for dialects in which specifying VARCHAR length // is mandatory in queries that modify the schema (CREATE TABLE / ADD COLUMN, etc). // Dialects that do not have such requirement may return 0, which should be interpreted so by the caller. - DefaultVarcharLen() uint + DefaultVarcharLen() int } // ------------------------------------------------------------------------------ @@ -174,6 +174,6 @@ func (d *nopDialect) IdentQuote() byte { return '"' } -func (d *nopDialect) DefaultVarcharLen() uint { +func (d *nopDialect) DefaultVarcharLen() int { return 0 } From 3335e0b9d6d3f424145e1f715223a0fffe773d9a Mon Sep 17 00:00:00 2001 From: Dmytro Solovei Date: Thu, 29 Dec 2022 11:21:43 +0100 Subject: [PATCH 7/7] feat: setError on attempt to set non-positive .Varchar() --- internal/dbtest/query_test.go | 4 ++++ internal/dbtest/testdata/snapshots/TestQuery-mariadb-157 | 1 + .../dbtest/testdata/snapshots/TestQuery-mssql2019-157 | 1 + internal/dbtest/testdata/snapshots/TestQuery-mysql5-157 | 1 + internal/dbtest/testdata/snapshots/TestQuery-mysql8-157 | 1 + internal/dbtest/testdata/snapshots/TestQuery-pg-157 | 1 + internal/dbtest/testdata/snapshots/TestQuery-pgx-157 | 1 + internal/dbtest/testdata/snapshots/TestQuery-sqlite-157 | 1 + query_table_create.go | 8 ++++++-- 9 files changed, 17 insertions(+), 2 deletions(-) create mode 100644 internal/dbtest/testdata/snapshots/TestQuery-mariadb-157 create mode 100644 internal/dbtest/testdata/snapshots/TestQuery-mssql2019-157 create mode 100644 internal/dbtest/testdata/snapshots/TestQuery-mysql5-157 create mode 100644 internal/dbtest/testdata/snapshots/TestQuery-mysql8-157 create mode 100644 internal/dbtest/testdata/snapshots/TestQuery-pg-157 create mode 100644 internal/dbtest/testdata/snapshots/TestQuery-pgx-157 create mode 100644 internal/dbtest/testdata/snapshots/TestQuery-sqlite-157 diff --git a/internal/dbtest/query_test.go b/internal/dbtest/query_test.go index c8dca0144..24cfd9195 100644 --- a/internal/dbtest/query_test.go +++ b/internal/dbtest/query_test.go @@ -982,6 +982,10 @@ func TestQuery(t *testing.T) { // Set default VARCHAR length to 10 return db.NewCreateTable().Model((*Model)(nil)).Varchar(10) }, + func(db *bun.DB) schema.QueryAppender { + // Non-positive VARCHAR length is illegal + return db.NewCreateTable().Model((*Model)(nil)).Varchar(-20) + }, } timeRE := regexp.MustCompile(`'2\d{3}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}(\.\d+)?(\+\d{2}:\d{2})?'`) diff --git a/internal/dbtest/testdata/snapshots/TestQuery-mariadb-157 b/internal/dbtest/testdata/snapshots/TestQuery-mariadb-157 new file mode 100644 index 000000000..7860dc22e --- /dev/null +++ b/internal/dbtest/testdata/snapshots/TestQuery-mariadb-157 @@ -0,0 +1 @@ +bun: illegal VARCHAR length: -20 diff --git a/internal/dbtest/testdata/snapshots/TestQuery-mssql2019-157 b/internal/dbtest/testdata/snapshots/TestQuery-mssql2019-157 new file mode 100644 index 000000000..7860dc22e --- /dev/null +++ b/internal/dbtest/testdata/snapshots/TestQuery-mssql2019-157 @@ -0,0 +1 @@ +bun: illegal VARCHAR length: -20 diff --git a/internal/dbtest/testdata/snapshots/TestQuery-mysql5-157 b/internal/dbtest/testdata/snapshots/TestQuery-mysql5-157 new file mode 100644 index 000000000..7860dc22e --- /dev/null +++ b/internal/dbtest/testdata/snapshots/TestQuery-mysql5-157 @@ -0,0 +1 @@ +bun: illegal VARCHAR length: -20 diff --git a/internal/dbtest/testdata/snapshots/TestQuery-mysql8-157 b/internal/dbtest/testdata/snapshots/TestQuery-mysql8-157 new file mode 100644 index 000000000..7860dc22e --- /dev/null +++ b/internal/dbtest/testdata/snapshots/TestQuery-mysql8-157 @@ -0,0 +1 @@ +bun: illegal VARCHAR length: -20 diff --git a/internal/dbtest/testdata/snapshots/TestQuery-pg-157 b/internal/dbtest/testdata/snapshots/TestQuery-pg-157 new file mode 100644 index 000000000..7860dc22e --- /dev/null +++ b/internal/dbtest/testdata/snapshots/TestQuery-pg-157 @@ -0,0 +1 @@ +bun: illegal VARCHAR length: -20 diff --git a/internal/dbtest/testdata/snapshots/TestQuery-pgx-157 b/internal/dbtest/testdata/snapshots/TestQuery-pgx-157 new file mode 100644 index 000000000..7860dc22e --- /dev/null +++ b/internal/dbtest/testdata/snapshots/TestQuery-pgx-157 @@ -0,0 +1 @@ +bun: illegal VARCHAR length: -20 diff --git a/internal/dbtest/testdata/snapshots/TestQuery-sqlite-157 b/internal/dbtest/testdata/snapshots/TestQuery-sqlite-157 new file mode 100644 index 000000000..7860dc22e --- /dev/null +++ b/internal/dbtest/testdata/snapshots/TestQuery-sqlite-157 @@ -0,0 +1 @@ +bun: illegal VARCHAR length: -20 diff --git a/query_table_create.go b/query_table_create.go index 22b4e6dee..0fe3013c7 100644 --- a/query_table_create.go +++ b/query_table_create.go @@ -3,6 +3,7 @@ package bun import ( "context" "database/sql" + "fmt" "sort" "strconv" "strings" @@ -18,12 +19,12 @@ type CreateTableQuery struct { temp bool ifNotExists bool - + // varchar changes the default length for VARCHAR columns. // Because some dialects require that length is always specified for VARCHAR type, // we will use the exact user-defined type if length is set explicitly, as in `bun:",type:varchar(5)"`, // but assume the new default length when it's omitted, e.g. `bun:",type:varchar"`. - varchar int + varchar int fks []schema.QueryWithArgs partitionBy schema.QueryWithArgs @@ -91,6 +92,9 @@ func (q *CreateTableQuery) IfNotExists() *CreateTableQuery { // Varchar sets default length for VARCHAR columns. func (q *CreateTableQuery) Varchar(n int) *CreateTableQuery { + if n <= 0 { + q.setErr(fmt.Errorf("bun: illegal VARCHAR length: %d", n)) + } q.varchar = n return q }