From 2a5b5c03a7922219b3f28665b81a96af98461582 Mon Sep 17 00:00:00 2001 From: abeizn Date: Tue, 18 Jul 2023 21:39:59 +0800 Subject: [PATCH 1/4] fix: migration bugs on postgres --- .../migrationhelper/migrationhelper.go | 8 +++++++- backend/impls/dalgorm/dalgorm.go | 19 +++++++++++++++++++ backend/impls/dalgorm/dalgorm_transaction.go | 17 +++++++++-------- 3 files changed, 35 insertions(+), 9 deletions(-) diff --git a/backend/helpers/migrationhelper/migrationhelper.go b/backend/helpers/migrationhelper/migrationhelper.go index 4de96c326c2..86cb341bf6a 100644 --- a/backend/helpers/migrationhelper/migrationhelper.go +++ b/backend/helpers/migrationhelper/migrationhelper.go @@ -263,13 +263,19 @@ func CopyTableColumns[S any, D any]( if err != nil { return errors.Default.Wrap(err, fmt.Sprintf("failed to load data from src table [%s]", srcTableName)) } - defer cursor.Close() + if !reflect.ValueOf(cursor).IsNil() { + defer cursor.Close() + } + batch, err := helper.NewBatchSave(basicRes, reflect.TypeOf(new(D)), 200, dstTableName) if err != nil { return errors.Default.Wrap(err, fmt.Sprintf("failed to instantiate BatchSave for table [%s]", srcTableName)) } defer batch.Close() + if reflect.ValueOf(cursor).IsNil() { + return nil + } for cursor.Next() { srcTable := new(S) err1 := db.Fetch(cursor, srcTable) diff --git a/backend/impls/dalgorm/dalgorm.go b/backend/impls/dalgorm/dalgorm.go index 5acde4faa4e..1b5f5aa0257 100644 --- a/backend/impls/dalgorm/dalgorm.go +++ b/backend/impls/dalgorm/dalgorm.go @@ -415,6 +415,18 @@ func (d *Dalgorm) IsDuplicationError(err error) bool { return strings.Contains(strings.ToLower(err.Error()), "duplicate") } +// IsCachedPlanError checks if the error is related to postgres cached query plan +// This error occurs occasionally in Postgres when reusing a cached query +// plan. It can be safely ignored since it does not actually affect results. +func (d *Dalgorm) IsCachedPlanError(err error) bool { + return strings.Contains(strings.ToLower(err.Error()), "cached plan must not change result type") +} + +// IsJsonOrderError checks if the error is related to postgres json ordering +func (d *Dalgorm) IsJsonOrderError(err error) bool { + return strings.Contains(err.Error(), "identify an ordering operator for type json") +} + // RawCursor (Deprecated) executes raw sql query and returns a database cursor func (d *Dalgorm) RawCursor(query string, params ...interface{}) (*sql.Rows, errors.Error) { rows, err := d.db.Raw(query, params...).Rows() @@ -436,5 +448,12 @@ func (d *Dalgorm) convertGormError(err error) errors.Error { if d.IsDuplicationError(err) { return errors.BadInput.WrapRaw(err) } + if d.IsJsonOrderError(err) { + return errors.BadInput.WrapRaw(err) + } + if d.IsCachedPlanError(err) { + return nil + } + panic(err) } diff --git a/backend/impls/dalgorm/dalgorm_transaction.go b/backend/impls/dalgorm/dalgorm_transaction.go index ed839e06a15..9ba88c93586 100644 --- a/backend/impls/dalgorm/dalgorm_transaction.go +++ b/backend/impls/dalgorm/dalgorm_transaction.go @@ -68,19 +68,20 @@ func (t *DalgormTransaction) LockTables(lockTables dal.LockTables) errors.Error } return t.Exec(fmt.Sprintf("LOCK TABLES %s", clause)) case "postgres": - clause := "" for _, lockTable := range lockTables { - if clause != "" { - clause += ", " - } - clause += lockTable.TableName() + var clause string if lockTable.Exclusive { - clause += " IN EXCLUSIVE MODE" + clause = "EXCLUSIVE" } else { - clause += " IN SHARE MODE" + clause = "SHARE" + } + stmt := fmt.Sprintf("LOCK TABLE %s IN %s MODE;", lockTable.TableName(), clause) + err := t.Exec(stmt) + if err != nil { + return err } } - return t.Exec(fmt.Sprintf("LOCK TABLE %s", clause)) + return nil default: panic(fmt.Errorf("unknown dialect %s", t.Dialect())) } From 852915062a55baa4bcc6af0bd372dfaf28d91b9b Mon Sep 17 00:00:00 2001 From: abeizn Date: Tue, 18 Jul 2023 22:42:03 +0800 Subject: [PATCH 2/4] fix: lock mutil-tables --- backend/impls/dalgorm/dalgorm_transaction.go | 32 +++++++++++++------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/backend/impls/dalgorm/dalgorm_transaction.go b/backend/impls/dalgorm/dalgorm_transaction.go index 9ba88c93586..1c3e87aefaf 100644 --- a/backend/impls/dalgorm/dalgorm_transaction.go +++ b/backend/impls/dalgorm/dalgorm_transaction.go @@ -19,6 +19,7 @@ package dalgorm import ( "fmt" + "strings" "github.com/apache/incubator-devlake/core/dal" "github.com/apache/incubator-devlake/core/errors" @@ -68,25 +69,34 @@ func (t *DalgormTransaction) LockTables(lockTables dal.LockTables) errors.Error } return t.Exec(fmt.Sprintf("LOCK TABLES %s", clause)) case "postgres": - for _, lockTable := range lockTables { - var clause string - if lockTable.Exclusive { - clause = "EXCLUSIVE" - } else { - clause = "SHARE" - } - stmt := fmt.Sprintf("LOCK TABLE %s IN %s MODE;", lockTable.TableName(), clause) - err := t.Exec(stmt) - if err != nil { - return err + var tables []string + var lockMode = "SHARE" + for _, t := range lockTables { + tables = append(tables, t.TableName()) + if t.Exclusive { + lockMode = "EXCLUSIVE" + break } } + stmt := fmt.Sprintf("LOCK TABLE %s IN %s MODE", strings.Join(tables, ","), lockMode) + err := t.Exec(stmt) + if err != nil { + return err + } return nil + default: panic(fmt.Errorf("unknown dialect %s", t.Dialect())) } } +func getLockMode(exclusive bool) string { + if exclusive { + return "EXCLUSIVE" + } + return "SHARE" +} + func (t *DalgormTransaction) UnlockTables() errors.Error { switch t.Dialect() { case "mysql": From e9a13d52cc257d2b50e67104668ebc8962351b0b Mon Sep 17 00:00:00 2001 From: abeizn Date: Tue, 18 Jul 2023 23:01:50 +0800 Subject: [PATCH 3/4] fix: lock mutil-tables --- backend/impls/dalgorm/dalgorm_transaction.go | 25 +++++++++----------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/backend/impls/dalgorm/dalgorm_transaction.go b/backend/impls/dalgorm/dalgorm_transaction.go index 1c3e87aefaf..fbade2b0a97 100644 --- a/backend/impls/dalgorm/dalgorm_transaction.go +++ b/backend/impls/dalgorm/dalgorm_transaction.go @@ -19,7 +19,6 @@ package dalgorm import ( "fmt" - "strings" "github.com/apache/incubator-devlake/core/dal" "github.com/apache/incubator-devlake/core/errors" @@ -69,22 +68,20 @@ func (t *DalgormTransaction) LockTables(lockTables dal.LockTables) errors.Error } return t.Exec(fmt.Sprintf("LOCK TABLES %s", clause)) case "postgres": - var tables []string - var lockMode = "SHARE" - for _, t := range lockTables { - tables = append(tables, t.TableName()) - if t.Exclusive { - lockMode = "EXCLUSIVE" - break + for _, lockTable := range lockTables { + var clause string + if lockTable.Exclusive { + clause = "EXCLUSIVE" + } else { + clause = "SHARE" + } + stmt := fmt.Sprintf("LOCK TABLE %s IN %s MODE;", lockTable.TableName(), clause) + err := t.Exec(stmt) + if err != nil { + return err } - } - stmt := fmt.Sprintf("LOCK TABLE %s IN %s MODE", strings.Join(tables, ","), lockMode) - err := t.Exec(stmt) - if err != nil { - return err } return nil - default: panic(fmt.Errorf("unknown dialect %s", t.Dialect())) } From 10ad66ae2c0477de269bea559ec589ea943776bb Mon Sep 17 00:00:00 2001 From: abeizn Date: Tue, 18 Jul 2023 23:11:19 +0800 Subject: [PATCH 4/4] fix: lock mutil-tables --- backend/impls/dalgorm/dalgorm_transaction.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/backend/impls/dalgorm/dalgorm_transaction.go b/backend/impls/dalgorm/dalgorm_transaction.go index fbade2b0a97..9ba88c93586 100644 --- a/backend/impls/dalgorm/dalgorm_transaction.go +++ b/backend/impls/dalgorm/dalgorm_transaction.go @@ -87,13 +87,6 @@ func (t *DalgormTransaction) LockTables(lockTables dal.LockTables) errors.Error } } -func getLockMode(exclusive bool) string { - if exclusive { - return "EXCLUSIVE" - } - return "SHARE" -} - func (t *DalgormTransaction) UnlockTables() errors.Error { switch t.Dialect() { case "mysql":