From ec5f5b294a3311709dbd39a285dd98660bd19415 Mon Sep 17 00:00:00 2001 From: Ariel Mashraki Date: Tue, 21 May 2024 07:39:04 +0300 Subject: [PATCH] sql/postgres: remove dynamic default comparison As dev-url is not a standard option, this comparison is not needed anymore. --- internal/integration/cockroach_test.go | 11 ++--- internal/integration/postgres_test.go | 12 ++--- sql/internal/sqlx/sqlx.go | 12 +++++ sql/internal/sqlx/sqlx_test.go | 1 + sql/postgres/diff.go | 63 ++++---------------------- 5 files changed, 30 insertions(+), 69 deletions(-) diff --git a/internal/integration/cockroach_test.go b/internal/integration/cockroach_test.go index 3fb0ddd947b..446632c8a7c 100644 --- a/internal/integration/cockroach_test.go +++ b/internal/integration/cockroach_test.go @@ -140,17 +140,14 @@ func TestCockroach_AddColumns(t *testing.T) { &schema.Column{Name: "d", Type: &schema.ColumnType{Type: &schema.DecimalType{T: "numeric", Precision: 10, Scale: 2}}, Default: &schema.Literal{V: "0.99"}}, &schema.Column{Name: "e", Type: &schema.ColumnType{Type: &schema.JSONType{T: "json"}}, Default: &schema.Literal{V: "'{}'"}}, &schema.Column{Name: "f", Type: &schema.ColumnType{Type: &schema.JSONType{T: "jsonb"}}, Default: &schema.Literal{V: "'1'"}}, - &schema.Column{Name: "g", Type: &schema.ColumnType{Type: &schema.FloatType{T: "float", Precision: 10}}, Default: &schema.Literal{V: "'1'"}}, - &schema.Column{Name: "h", Type: &schema.ColumnType{Type: &schema.FloatType{T: "float", Precision: 30}}, Default: &schema.Literal{V: "'1'"}}, - &schema.Column{Name: "i", Type: &schema.ColumnType{Type: &schema.FloatType{T: "float", Precision: 53}}, Default: &schema.Literal{V: "1"}}, + &schema.Column{Name: "g", Type: &schema.ColumnType{Type: &schema.FloatType{T: "float", Precision: 10}}, Default: &schema.Literal{V: "1.0"}}, + &schema.Column{Name: "h", Type: &schema.ColumnType{Type: &schema.FloatType{T: "float", Precision: 30}}, Default: &schema.Literal{V: "1.0"}}, + &schema.Column{Name: "i", Type: &schema.ColumnType{Type: &schema.FloatType{T: "float", Precision: 53}}, Default: &schema.Literal{V: "1.0"}}, &schema.Column{Name: "j", Type: &schema.ColumnType{Type: &postgres.SerialType{T: "serial"}}}, &schema.Column{Name: "m", Type: &schema.ColumnType{Type: &schema.BoolType{T: "boolean"}, Null: true}, Default: &schema.Literal{V: "false"}}, - &schema.Column{Name: "n", Type: &schema.ColumnType{Type: &schema.SpatialType{T: "geometry"}, Null: true}, Default: &schema.Literal{V: "'POINT(1 2)'"}}, - &schema.Column{Name: "o", Type: &schema.ColumnType{Type: &schema.SpatialType{T: "geometry"}, Null: true}, Default: &schema.Literal{V: "'LINESTRING(0 0, 1440 900)'"}}, - &schema.Column{Name: "q", Type: &schema.ColumnType{Type: &postgres.ArrayType{Type: &schema.StringType{T: "text"}, T: "text[]"}, Null: true}, Default: &schema.Literal{V: "'{}'"}}, ) changes := t.diff(t.loadUsers(), usersT) - require.Len(t, changes, 14) + require.Len(t, changes, 11) t.migrate(&schema.ModifyTable{T: usersT, Changes: changes}) ensureNoChange(t, usersT) }) diff --git a/internal/integration/postgres_test.go b/internal/integration/postgres_test.go index e6340e59828..c69af32071b 100644 --- a/internal/integration/postgres_test.go +++ b/internal/integration/postgres_test.go @@ -195,12 +195,12 @@ func TestPostgres_AddColumns(t *testing.T) { &schema.Column{Name: "h", Type: &schema.ColumnType{Type: &schema.FloatType{T: "float", Precision: 30}}, Default: &schema.Literal{V: "'1'"}}, &schema.Column{Name: "i", Type: &schema.ColumnType{Type: &schema.FloatType{T: "float", Precision: 53}}, Default: &schema.Literal{V: "1"}}, &schema.Column{Name: "j", Type: &schema.ColumnType{Type: &postgres.SerialType{T: "serial"}}}, - &schema.Column{Name: "k", Type: &schema.ColumnType{Type: &postgres.CurrencyType{T: "money"}}, Default: &schema.Literal{V: "'100'"}}, - &schema.Column{Name: "l", Type: &schema.ColumnType{Type: &postgres.CurrencyType{T: "money"}, Null: true}, Default: &schema.RawExpr{X: "'52093.89'::money"}}, + &schema.Column{Name: "k", Type: &schema.ColumnType{Type: &postgres.CurrencyType{T: "money"}}, Default: &schema.RawExpr{X: "'$100.00'::money"}}, + &schema.Column{Name: "l", Type: &schema.ColumnType{Type: &postgres.CurrencyType{T: "money"}, Null: true}, Default: &schema.RawExpr{X: "'$52,093.89'::money"}}, &schema.Column{Name: "m", Type: &schema.ColumnType{Type: &schema.BoolType{T: "boolean"}, Null: true}, Default: &schema.Literal{V: "false"}}, &schema.Column{Name: "n", Type: &schema.ColumnType{Type: &schema.SpatialType{T: "point"}, Null: true}, Default: &schema.Literal{V: "'(1,2)'"}}, &schema.Column{Name: "o", Type: &schema.ColumnType{Type: &schema.SpatialType{T: "line"}, Null: true}, Default: &schema.Literal{V: "'{1,2,3}'"}}, - &schema.Column{Name: "p", Type: &schema.ColumnType{Type: &postgres.UserDefinedType{T: "hstore"}, Null: true}, Default: &schema.RawExpr{X: "'a => 1'"}}, + &schema.Column{Name: "p", Type: &schema.ColumnType{Type: &postgres.UserDefinedType{T: "hstore"}, Null: true}, Default: &schema.RawExpr{X: `'"a"=>"1"'::public.hstore`}}, &schema.Column{Name: "q", Type: &schema.ColumnType{Type: &postgres.ArrayType{Type: &schema.StringType{T: "text"}, T: "text[]"}, Null: true}, Default: &schema.Literal{V: "'{}'"}}, ) changes := t.diff(t.loadUsers(), usersT) @@ -264,12 +264,8 @@ func TestPostgres_ColumnArray(t *testing.T) { t.migrate(&schema.ModifyTable{T: usersT, Changes: changes}) ensureNoChange(t, usersT) - // Check default. - usersT.Columns[2].Default = &schema.RawExpr{X: "ARRAY[1]"} - ensureNoChange(t, usersT) - // Change default. - usersT.Columns[2].Default = &schema.RawExpr{X: "ARRAY[1,2]"} + usersT.Columns[2].Default = &schema.Literal{V: "'{1,2}'"} changes = t.diff(t.loadUsers(), usersT) require.Len(t, changes, 1) t.migrate(&schema.ModifyTable{T: usersT, Changes: changes}) diff --git a/sql/internal/sqlx/sqlx.go b/sql/internal/sqlx/sqlx.go index 5e312890b11..ac16a606d94 100644 --- a/sql/internal/sqlx/sqlx.go +++ b/sql/internal/sqlx/sqlx.go @@ -497,6 +497,18 @@ func (b *Builder) WrapIndent(f func(b *Builder)) *Builder { }) } +// WrapIndentErr is like WrapErr but with extra level of indentation. +func (b *Builder) WrapIndentErr(f func(b *Builder) error) error { + var err error + b.Wrap(func(b *Builder) { + b.IndentIn() + err = f(b) + b.IndentOut() + b.NL() + }) + return err +} + // Clone returns a duplicate of the builder. func (b *Builder) Clone() *Builder { return &Builder{ diff --git a/sql/internal/sqlx/sqlx_test.go b/sql/internal/sqlx/sqlx_test.go index d22478a57c6..c7fac7fc7c0 100644 --- a/sql/internal/sqlx/sqlx_test.go +++ b/sql/internal/sqlx/sqlx_test.go @@ -63,6 +63,7 @@ func TestBuilder(t *testing.T) { // WrapErr. require.EqualError(t, b.WrapErr(func(*Builder) error { return errors.New("oops") }), "oops") + require.EqualError(t, b.WrapIndentErr(func(*Builder) error { return errors.New("oops") }), "oops") } func TestBuilder_Qualifier(t *testing.T) { diff --git a/sql/postgres/diff.go b/sql/postgres/diff.go index 6e7d59eb869..a9db3b00654 100644 --- a/sql/postgres/diff.go +++ b/sql/postgres/diff.go @@ -5,7 +5,6 @@ package postgres import ( - "context" "errors" "fmt" "reflect" @@ -106,24 +105,7 @@ func (d *diff) defaultChanged(from, to *schema.Column) (bool, error) { if ok1 != ok2 { return true, nil } - if !ok1 && !ok2 || trimCast(d1) == trimCast(d2) || quote(d1) == quote(d2) { - return false, nil - } - var ( - _, fromX = from.Default.(*schema.RawExpr) - _, toX = to.Default.(*schema.RawExpr) - ) - // In case one of the DEFAULT values is an expression, and a database connection is - // available (not DefaultDiff), we use the database to compare in case of mismatch. - // - // SELECT ARRAY[1] = '{1}'::int[] - // SELECT lower('X'::text) = lower('X') - // - if (fromX || toX) && d.conn.ExecQuerier != nil { - equals, err := d.defaultEqual(from.Default, to.Default) - return !equals, err - } - return true, nil + return ok1 && (trimCast(d1) != trimCast(d2) && quote(d1) != quote(d2)), nil } // generatedChanged reports if the generated expression of a column was changed. @@ -385,37 +367,6 @@ func trimSchema(t string, ns string) string { return strings.TrimPrefix(t, fmt.Sprintf("%s.", ns)) } -// defaultEqual reports if the DEFAULT values x and y -// equal according to the database engine. -func (d *diff) defaultEqual(from, to schema.Expr) (bool, error) { - var ( - b bool - d1, d2 string - ) - switch from := from.(type) { - case *schema.Literal: - d1 = quote(from.V) - case *schema.RawExpr: - d1 = from.X - } - switch to := to.(type) { - case *schema.Literal: - d2 = quote(to.V) - case *schema.RawExpr: - d2 = to.X - } - // The DEFAULT expressions are safe to be inlined in the SELECT - // statement same as we inline them in the CREATE TABLE statement. - rows, err := d.QueryContext(context.Background(), fmt.Sprintf("SELECT %s = %s", d1, d2)) - if err != nil { - return false, err - } - if err := sqlx.ScanOne(rows, &b); err != nil { - return false, err - } - return b, nil -} - // Default IDENTITY attributes. const ( defaultIdentityGen = "BY DEFAULT" @@ -539,12 +490,16 @@ func excludeConst(attrs []schema.Attr) (*Constraint, bool) { } func trimCast(s string) string { - i := strings.LastIndex(s, "::") - if i == -1 { + var i, k int + if j := strings.LastIndex(s, ":::"); j != -1 { + i, k = j, 3 // Cockroach cast. + } else if j := strings.LastIndex(s, "::"); j != -1 { + i, k = j, 2 // Postgres cast. + } else { return s } - for _, r := range s[i+2:] { - if r != ' ' && !unicode.IsLetter(r) { + for _, r := range s[i+k:] { + if r != ' ' && !unicode.IsLetter(r) && !unicode.IsDigit(r) { return s } }