klesh commented on code in PR #3473:
URL: 
https://github.com/apache/incubator-devlake/pull/3473#discussion_r1002971755


##########
helpers/migrationhelper/migrationhelper.go:
##########
@@ -42,6 +43,134 @@ func AutoMigrateTables(basicRes core.BasicRes, dst 
...interface{}) errors.Error
        return nil
 }
 
+// ChangeColumnsType change some Columns type for one table
+func ChangeColumnsType(
+       basicRes core.BasicRes,
+       script core.MigrationScript,
+       newTable core.Tabler,
+       Columns []string,

Review Comment:
   Why is it PascalCased?



##########
helpers/migrationhelper/migrationhelper.go:
##########
@@ -42,6 +43,134 @@ func AutoMigrateTables(basicRes core.BasicRes, dst 
...interface{}) errors.Error
        return nil
 }
 
+// ChangeColumnsType change some Columns type for one table

Review Comment:
   change **the type of specified columns for the table**



##########
helpers/migrationhelper/migrationhelper.go:
##########
@@ -42,6 +43,134 @@ func AutoMigrateTables(basicRes core.BasicRes, dst 
...interface{}) errors.Error
        return nil
 }
 
+// ChangeColumnsType change some Columns type for one table
+func ChangeColumnsType(
+       basicRes core.BasicRes,
+       script core.MigrationScript,
+       newTable core.Tabler,
+       Columns []string,
+       update func(tmpColumnsNames []string) errors.Error,
+) (err errors.Error) {
+       db := basicRes.GetDal()
+       tmpColumnsNames := make([]string, len(Columns))
+       for i, v := range Columns {
+               tmpColumnsNames[i] = fmt.Sprintf("%s_%s", v, hashScript(script))
+               err = db.RenameColumn(newTable.TableName(), v, 
tmpColumnsNames[i])
+               if err != nil {
+                       return err
+               }
+
+               defer func(tmpColumnName string, ColumnsName string) {
+                       if err != nil {
+                               err1 := db.RenameColumn(newTable.TableName(), 
tmpColumnName, ColumnsName)
+                               if err1 != nil {
+                                       err = errors.Default.Wrap(err, 
fmt.Sprintf("RollBack by RenameColum failed.Relevant data needs to be repaired 
manually.%s", err1.Error()))
+                               }
+                       }
+               }(tmpColumnsNames[i], v)
+       }
+
+       err = db.AutoMigrate(newTable)
+       if err != nil {
+               return errors.Default.Wrap(err, "AutoMigrate for Add Colume 
Error")
+       }
+
+       defer func() {
+               if err != nil {
+                       err1 := db.DropColumns(newTable.TableName(), Columns...)
+                       if err1 != nil {
+                               err = errors.Default.Wrap(err, 
fmt.Sprintf("RollBack by DropColume failed.Relevant data needs to be repaired 
manually.%s", err1.Error()))
+                       }
+               }
+       }()
+
+       if update == nil {
+               params := make([]interface{}, 0, len(Columns)*2+1)
+               params = append(params, clause.Table{Name: 
newTable.TableName()})
+               updateStr := "UPDATE ? SET "
+               for i, v := range Columns {
+                       if i > 0 {
+                               updateStr += " AND "
+                       }
+                       updateStr += "? = ?"
+                       params = append(params, clause.Column{Name: v})
+                       params = append(params, clause.Column{Name: 
tmpColumnsNames[i]})
+               }
+               err = db.Exec(updateStr, params...)
+       } else {
+               err = update(tmpColumnsNames)
+       }
+       if err != nil {
+               return err
+       }
+
+       err = db.DropColumns(newTable.TableName(), tmpColumnsNames...)
+       if err != nil {
+               return err
+       }
+
+       return nil
+}
+
+// ChangeColumnsTypeOneByOne change some Columns type for one table
+func ChangeColumnsTypeOneByOne[S any, D core.Tabler](

Review Comment:
   How about `TransformColumns` because it acts much like the `TransformTable`



##########
helpers/migrationhelper/migrationhelper.go:
##########
@@ -42,6 +43,134 @@ func AutoMigrateTables(basicRes core.BasicRes, dst 
...interface{}) errors.Error
        return nil
 }
 
+// ChangeColumnsType change some Columns type for one table
+func ChangeColumnsType(
+       basicRes core.BasicRes,
+       script core.MigrationScript,
+       newTable core.Tabler,
+       Columns []string,
+       update func(tmpColumnsNames []string) errors.Error,
+) (err errors.Error) {
+       db := basicRes.GetDal()
+       tmpColumnsNames := make([]string, len(Columns))
+       for i, v := range Columns {
+               tmpColumnsNames[i] = fmt.Sprintf("%s_%s", v, hashScript(script))
+               err = db.RenameColumn(newTable.TableName(), v, 
tmpColumnsNames[i])
+               if err != nil {
+                       return err
+               }
+
+               defer func(tmpColumnName string, ColumnsName string) {
+                       if err != nil {
+                               err1 := db.RenameColumn(newTable.TableName(), 
tmpColumnName, ColumnsName)
+                               if err1 != nil {
+                                       err = errors.Default.Wrap(err, 
fmt.Sprintf("RollBack by RenameColum failed.Relevant data needs to be repaired 
manually.%s", err1.Error()))
+                               }
+                       }
+               }(tmpColumnsNames[i], v)
+       }
+
+       err = db.AutoMigrate(newTable)
+       if err != nil {
+               return errors.Default.Wrap(err, "AutoMigrate for Add Colume 
Error")
+       }
+
+       defer func() {
+               if err != nil {
+                       err1 := db.DropColumns(newTable.TableName(), Columns...)
+                       if err1 != nil {
+                               err = errors.Default.Wrap(err, 
fmt.Sprintf("RollBack by DropColume failed.Relevant data needs to be repaired 
manually.%s", err1.Error()))
+                       }
+               }
+       }()
+
+       if update == nil {
+               params := make([]interface{}, 0, len(Columns)*2+1)
+               params = append(params, clause.Table{Name: 
newTable.TableName()})
+               updateStr := "UPDATE ? SET "
+               for i, v := range Columns {
+                       if i > 0 {
+                               updateStr += " AND "
+                       }
+                       updateStr += "? = ?"
+                       params = append(params, clause.Column{Name: v})
+                       params = append(params, clause.Column{Name: 
tmpColumnsNames[i]})
+               }
+               err = db.Exec(updateStr, params...)
+       } else {
+               err = update(tmpColumnsNames)
+       }
+       if err != nil {
+               return err
+       }
+
+       err = db.DropColumns(newTable.TableName(), tmpColumnsNames...)
+       if err != nil {
+               return err
+       }
+
+       return nil
+}
+
+// ChangeColumnsTypeOneByOne change some Columns type for one table
+func ChangeColumnsTypeOneByOne[S any, D core.Tabler](

Review Comment:
   Is `core.Tabler` a must? Can we ask for the `tableName` and `any` struct 
just like the `TransformTable` method? It would make much sense because the 
`tableName` is never change.
   With current construct, the Developer would be so confuse to understand 
which struct should implement the `Tabler` interface. 



##########
helpers/migrationhelper/migrationhelper.go:
##########
@@ -42,6 +43,134 @@ func AutoMigrateTables(basicRes core.BasicRes, dst 
...interface{}) errors.Error
        return nil
 }
 
+// ChangeColumnsType change some Columns type for one table
+func ChangeColumnsType(
+       basicRes core.BasicRes,
+       script core.MigrationScript,
+       newTable core.Tabler,
+       Columns []string,
+       update func(tmpColumnsNames []string) errors.Error,
+) (err errors.Error) {
+       db := basicRes.GetDal()
+       tmpColumnsNames := make([]string, len(Columns))
+       for i, v := range Columns {
+               tmpColumnsNames[i] = fmt.Sprintf("%s_%s", v, hashScript(script))
+               err = db.RenameColumn(newTable.TableName(), v, 
tmpColumnsNames[i])
+               if err != nil {
+                       return err
+               }
+
+               defer func(tmpColumnName string, ColumnsName string) {
+                       if err != nil {
+                               err1 := db.RenameColumn(newTable.TableName(), 
tmpColumnName, ColumnsName)
+                               if err1 != nil {
+                                       err = errors.Default.Wrap(err, 
fmt.Sprintf("RollBack by RenameColum failed.Relevant data needs to be repaired 
manually.%s", err1.Error()))
+                               }
+                       }
+               }(tmpColumnsNames[i], v)
+       }
+
+       err = db.AutoMigrate(newTable)
+       if err != nil {
+               return errors.Default.Wrap(err, "AutoMigrate for Add Colume 
Error")
+       }
+
+       defer func() {
+               if err != nil {
+                       err1 := db.DropColumns(newTable.TableName(), Columns...)
+                       if err1 != nil {
+                               err = errors.Default.Wrap(err, 
fmt.Sprintf("RollBack by DropColume failed.Relevant data needs to be repaired 
manually.%s", err1.Error()))
+                       }
+               }
+       }()
+
+       if update == nil {
+               params := make([]interface{}, 0, len(Columns)*2+1)
+               params = append(params, clause.Table{Name: 
newTable.TableName()})
+               updateStr := "UPDATE ? SET "
+               for i, v := range Columns {
+                       if i > 0 {
+                               updateStr += " AND "
+                       }
+                       updateStr += "? = ?"
+                       params = append(params, clause.Column{Name: v})
+                       params = append(params, clause.Column{Name: 
tmpColumnsNames[i]})
+               }
+               err = db.Exec(updateStr, params...)
+       } else {
+               err = update(tmpColumnsNames)
+       }
+       if err != nil {
+               return err
+       }
+
+       err = db.DropColumns(newTable.TableName(), tmpColumnsNames...)
+       if err != nil {
+               return err
+       }
+
+       return nil
+}
+
+// ChangeColumnsTypeOneByOne change some Columns type for one table
+func ChangeColumnsTypeOneByOne[S any, D core.Tabler](
+       basicRes core.BasicRes,
+       script core.MigrationScript,
+       Columns []string,
+       update func(src *S) (D, errors.Error),

Review Comment:
   `update` is not a good name here, consider `transform`



##########
helpers/migrationhelper/migrationhelper.go:
##########
@@ -42,6 +43,134 @@ func AutoMigrateTables(basicRes core.BasicRes, dst 
...interface{}) errors.Error
        return nil
 }
 
+// ChangeColumnsType change some Columns type for one table
+func ChangeColumnsType(
+       basicRes core.BasicRes,
+       script core.MigrationScript,
+       newTable core.Tabler,

Review Comment:
   newTable?



##########
helpers/migrationhelper/migrationhelper.go:
##########
@@ -42,6 +43,134 @@ func AutoMigrateTables(basicRes core.BasicRes, dst 
...interface{}) errors.Error
        return nil
 }
 
+// ChangeColumnsType change some Columns type for one table
+func ChangeColumnsType(
+       basicRes core.BasicRes,
+       script core.MigrationScript,
+       newTable core.Tabler,
+       Columns []string,
+       update func(tmpColumnsNames []string) errors.Error,
+) (err errors.Error) {
+       db := basicRes.GetDal()
+       tmpColumnsNames := make([]string, len(Columns))
+       for i, v := range Columns {
+               tmpColumnsNames[i] = fmt.Sprintf("%s_%s", v, hashScript(script))
+               err = db.RenameColumn(newTable.TableName(), v, 
tmpColumnsNames[i])
+               if err != nil {
+                       return err
+               }
+
+               defer func(tmpColumnName string, ColumnsName string) {
+                       if err != nil {
+                               err1 := db.RenameColumn(newTable.TableName(), 
tmpColumnName, ColumnsName)
+                               if err1 != nil {
+                                       err = errors.Default.Wrap(err, 
fmt.Sprintf("RollBack by RenameColum failed.Relevant data needs to be repaired 
manually.%s", err1.Error()))
+                               }
+                       }
+               }(tmpColumnsNames[i], v)
+       }
+
+       err = db.AutoMigrate(newTable)
+       if err != nil {
+               return errors.Default.Wrap(err, "AutoMigrate for Add Colume 
Error")
+       }
+
+       defer func() {
+               if err != nil {
+                       err1 := db.DropColumns(newTable.TableName(), Columns...)
+                       if err1 != nil {
+                               err = errors.Default.Wrap(err, 
fmt.Sprintf("RollBack by DropColume failed.Relevant data needs to be repaired 
manually.%s", err1.Error()))
+                       }
+               }
+       }()
+
+       if update == nil {
+               params := make([]interface{}, 0, len(Columns)*2+1)
+               params = append(params, clause.Table{Name: 
newTable.TableName()})
+               updateStr := "UPDATE ? SET "
+               for i, v := range Columns {
+                       if i > 0 {
+                               updateStr += " AND "
+                       }
+                       updateStr += "? = ?"
+                       params = append(params, clause.Column{Name: v})
+                       params = append(params, clause.Column{Name: 
tmpColumnsNames[i]})
+               }
+               err = db.Exec(updateStr, params...)
+       } else {
+               err = update(tmpColumnsNames)
+       }
+       if err != nil {
+               return err
+       }
+
+       err = db.DropColumns(newTable.TableName(), tmpColumnsNames...)
+       if err != nil {
+               return err
+       }
+
+       return nil
+}
+
+// ChangeColumnsTypeOneByOne change some Columns type for one table
+func ChangeColumnsTypeOneByOne[S any, D core.Tabler](
+       basicRes core.BasicRes,
+       script core.MigrationScript,
+       Columns []string,

Review Comment:
   Same as above



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to