Some review notes

[shlomi-noach]

In:

go-dump/go/utils/table.go

Line 16 in 7225b1c

uniqueKey string

go-dump/go/utils/table.go

Line 106 in 7225b1c

this.uniqueKey = fName

Does this imply you're limiting to a single UNIQUE key for a table?
I'd advise against using DESC / SHOW COLUMNS output, and instead go deep into INFORMATION_SCHEMA.

---

In:

go-dump/go/utils/utils.go

Lines 104 to 106 in 7225b1c

	query := fmt.Sprintf("SELECT TABLE_SCHEMA, TABLE_NAME "+
	"FROM information_schema.TABLES WHERE TABLE_SCHEMA IN('%s') AND TABLE_TYPE ='BASE TABLE' AND "+
	"NOT (TABLE_SCHEMA = 'mysql' AND TABLE_NAME NOT IN ( 'slow_log' , 'general_log'))", strings.Join(databases, "','"))

Don't use "something" + " something " + " something"

Instead, use backticks for multi line strings.

---

A session can only lock multiple tables in a single statement. Multiple statements won't work:

go-dump/go/utils/taskmanager.go

Line 88 in 7225b1c

if err := task.Table.Lock(this.DB); err != nil {

https://dev.mysql.com/doc/refman/5.7/en/lock-tables.html

A session that requires locks must acquire all the locks that it needs in a single LOCK TABLES statement.

---

go-dump/go/utils/taskmanager.go

Line 136 in 7225b1c

if len(cols) == 5 {

try an avoid assumptions like this. Maybe tomorrow, or in MariaDB, or in PerconaServer, there will be 6 columns.

---

go-dump/go/utils/taskmanager.go

Line 204 in 7225b1c

if lockTables == true {

  if lockTables {

---

go-dump/go/cmd/go-dump/main.go

Line 43 in 7225b1c

func GetMySQLConnection(host *MySQLHost, credentials *MySQLCredentials) (*sql.DB, error) {

avoid having such function in the main file. Move this to some utils.

---

[martinarrieta]

Shlomi,
Thank you for the review, I really appreciate your comments.

I have pushed the modifications to for some of your recommendations, I will continue with the other changes later.

For the purpose of this tool I just need one numeric unique key (I need to check for "numeric"). I'm also still thinking in the way to include compound keys, because for this case, I just need the leftmost field because in the worse case, what I will get is larger chunk (if the key on the left has some duplicates) but I will never write duplicate rows in the dump. So I'm not sure if I should include the information.

I also need to work to get the master data information from any mysql version, also the master information in case that you are making the backups from the slave.

Regards,

Martin.

[shlomi-noach]

@martinarrieta allow me to suggest pushing code by way of branches & pull requests. That way your changes are made visible and are reviewable. Otherwise it's just an endless stream of commits that cannot be easily followed.

[martinarrieta]

@shlomi-noach I have added you as a collaborator.. that should be enough..

Thanks !

[shlomi-noach]

I have added you as a collaborator.. that should be enough..

@martinarrieta thanks -- though I'm not sure what my role would be as a collaborator, or what it was you wished to achieve?