Repository: cloudspannerecosystem/harbourbridge (go)

Issue:
Handle data conversion of non-boolean values for MySQL TINYINT(1)/BOOLEAN
NOTE: TINYINT(1) is a synonym for BOOL and BOOLEAN.

MySQL does not require values for BOOLEAN to actually be boolean values.
Instead, it allows any legal value for tinyint to be stored in a BOOLEAN i.e. -128 to 127.

At the moment, HarbourBridge generates an error and rejects rows where boolean columns have non-boolean values.

Proposed resolution: instead of generating an error, convert integer values to booleans using the standard heuristic: 0 is false and non-zero is true. 


Pull request description:
#136: fix: Handle non-boolean values for boolean columns.
This PR changes the behaviour during data conversion for boolean columns with non-boolean values: instead of generating an error, we now try to convert an int to a bool.

Old behaviour: HarbourBridge generates an error and rejects rows when boolean columns have non-boolean values.

New behaviour: We try to parse the value as an int64. If that succeeds, then we convert the int to a bool (0 maps to false, everything else maps to true), and we log an "unexpected" event detailing the conversion. If the conversion to int64 fails, the behavior is the same as current behaviour (we log an error and drop the row).

Closes #136 

Patch:
diff --git a/frontend/js/config/constantData.js b/frontend/js/config/constantData.js
index 23c6ef2..0418766 100644
--- a/frontend/js/config/constantData.js
+++ b/frontend/js/config/constantData.js
@@ -83,17 +83,17 @@ export const MAIN_PAGE_STATIC_CONTENT = {
 export const TAB_CONFIG_DATA = [
   {
     id: "report",
-    text: "Conversion Report",
+    text: "Schema",
   },
 
   {
     id: "ddl",
-    text: "DDL Statements",
+    text: "DDL",
   },
   
   {
     id: "summary",
-    text: "Summary Report",
+    text: "Warnings",
   },
 ];
 
diff --git a/frontend/js/pages/Instructions/Instructions.page.js b/frontend/js/pages/Instructions/Instructions.page.js
index 22c7982..3864f62 100644
--- a/frontend/js/pages/Instructions/Instructions.page.js
+++ b/frontend/js/pages/Instructions/Instructions.page.js
@@ -36,29 +36,29 @@ class Instructions extends HTMLElement {
                 <br>
                 <h3 class="instructions-main-heading">2 &nbsp; &nbsp; &nbsp;Key Features of UI</h3>
                 <ul>
-                    <li>- Connecting to a new database</li>
-                    <li>- Load dump file</li>
-                    <li>- Load session file</li>
-                    <li>- Storing session for each conversion</li>
-                    <li>- Edit data type globally for each table in schema</li>
-                    <li>- Edit data type, column name, constraint for a particular table</li>
-                    <li>- Edit foreign key and secondary index name</li>
-                    <li>- Drop a column from a table</li>
-                    <li>- Drop foreign key from a table</li>
-                    <li>- Drop secondary index from a table</li>
-                    <li>- Convert foreign key into interleave table</li>
-                    <li>- Search a table</li>
-                    <li>- Download schema, report and session files</li>
+                    <li> Connecting to a new database</li>
+                    <li> Load dump file</li>
+                    <li> Load session file</li>
+                    <li> Storing session for each conversion</li>
+                    <li> Edit data type globally for each table in schema</li>
+                    <li> Edit data type, column name, constraint for a particular table</li>
+                    <li> Edit foreign key and secondary index name</li>
+                    <li> Drop a column from a table</li>
+                    <li> Drop foreign key from a table</li>
+                    <li> Drop secondary index from a table</li>
+                    <li> Convert foreign key into interleave table</li>
+                    <li> Search a table</li>
+                    <li> Download schema, report and session files</li>
                 </ul>
                 <br>
 
                 <h3 class="instructions-main-heading">3 &nbsp; &nbsp; &nbsp;UI Setup</h3>
                 <ul>
-                    <li>- Install go in local</li>
-                    <li>- Clone harbourbridge project and run following command in the terminal: <br>
+                    <li> Install go in local</li>
+                    <li> Clone harbourbridge project and run following command in the terminal: <br>
                         <span class="instructions-command">go run main.go --web</span>
                     </li>
-                    <li>- Open <span class="instructions-command">http://localhost:8080</span>in browser</li>
+                    <li> Open <span class="instructions-command">http://localhost:8080</span>in browser</li>
                 </ul>
                 <br>
 
@@ -66,8 +66,8 @@ class Instructions extends HTMLElement {
                     source database</h3>
                 <h4 class="instructions-sub-heading">4.1 &nbsp; &nbsp; &nbsp;Connect to Database</h4>
                 <ul>
-                    <li>- Enter database details in connect to database dialog box</li>
-                    <li>- Input Fields: database type, database host, database port, database user, database name, database password
+                    <li> Enter database details in connect to database dialog box</li>
+                    <li> Input Fields: database type, database host, database port, database user, database name, database password
                     </li>
                 </ul>
                 <br>
@@ -78,8 +78,8 @@ class Instructions extends HTMLElement {
 
                 <h4 class="instructions-sub-heading">4.2 &nbsp; &nbsp; &nbsp;Load Database Dump</h4>
                 <ul>
-                    <li>- Enter dump file path in load database dialog box</li>
-                    <li>- Input Fields: database type, file path</li>
+                    <li> Enter dump file path in load database dialog box</li>
+                    <li> Input Fields: database type, file path</li>
                 </ul>
                 <br>
                 <img class="instructions-img-width" src='userManualImages/loadDumpFile.png'>
@@ -87,8 +87,8 @@ class Instructions extends HTMLElement {
 
                 <h4 class="instructions-sub-heading">4.3 &nbsp; &nbsp; &nbsp;Import Schema File</h4>
                 <ul>
-                    <li>- Enter session file path in load session dialog box</li>
-                    <li>- Input Fields: database type, session file path</li>
+                    <li> Enter session file path in load session dialog box</li>
+                    <li> Input Fields: database type, session file path</li>
                 </ul>
                 <br>
                 <img class="instructions-img-width" src='userManualImages/loadSessionFile.png'>
@@ -96,7 +96,7 @@ class Instructions extends HTMLElement {
 
                 <h3 class="instructions-main-heading">5 &nbsp; &nbsp; &nbsp;Session Table</h3>
                 <ul>
-                    <li>- Session table is used to store the previous sessions of schema conversion</li>
+                    <li> Session table is used to store the previous sessions of schema conversion</li>
                 </ul>
                 <br>
                 <img class="instructions-img-width" src='userManualImages/sessionTable.png'>
@@ -104,9 +104,9 @@ class Instructions extends HTMLElement {
 
                 <h3 class="instructions-main-heading">6 &nbsp; &nbsp; &nbsp;Edit Global Data Type</h3>
                 <ul>
-                    <li>- Click on edit global data type button on the screen</li>
-                    <li>- Select required spanner data type from the dropdown available for each source data type</li>
-                    <li>- Click on next button after making all the changes</li>
+                    <li> Click on edit global data type button on the screen</li>
+                    <li> Select required spanner data type from the dropdown available for each source data type</li>
+                    <li> Click on next button after making all the changes</li>
                 </ul>
                 <br>
                 <img class="instructions-img-width" src='userManualImages/globalDataTypeMapping.png'>
@@ -115,15 +115,15 @@ class Instructions extends HTMLElement {
                 <h3 class="instructions-main-heading">7 &nbsp; &nbsp; &nbsp;Edit Spanner Schema for a
                     particular table</h3>
                 <ul>
-                    <li>- Expand any table</li>
-                    <li>- Click on edit spanner schema button</li>
-                    <li>- Edit column name/ data type/ constraint of spanner schema</li>
-                    <li>- Edit name of secondary index or foreign key</li>
-                    <li>- Select to convert foreign key to interleave or use as is (if option is available)</li>
-                    <li>- Drop a column by unselecting any checkbox</li>
-                    <li>- Drop a foreign key or secondary index by expanding foreign keys or secondary indexes tab inside table</li>
-                    <li>- Click on save changes button to save the changes</li>
-                    <li>- If current table is involved in foreign key/secondary indexes relationship with other table then user will
+                    <li> Expand any table</li>
+                    <li> Click on edit spanner schema button</li>
+                    <li> Edit column name/ data type/ constraint of spanner schema</li>
+                    <li> Edit name of secondary index or foreign key</li>
+                    <li> Select to convert foreign key to interleave or use as is (if option is available)</li>
+                    <li> Drop a column by unselecting any checkbox</li>
+                    <li> Drop a foreign key or secondary index by expanding foreign keys or secondary indexes tab inside table</li>
+                    <li> Click on save changes button to save the changes</li>
+                    <li> If current table is involved in foreign key/secondary indexes relationship with other table then user will
                         be prompt to delete foreign key or secondary indexes and then proceed with save changes</li>
                 </ul>
                 <br>
@@ -141,9 +141,9 @@ class Instructions extends HTMLElement {
 
                 <h3 class="instructions-main-heading">8 &nbsp; &nbsp; &nbsp;Download Session File</h3>
                 <ul>
-                    <li>- Save all the changes done in spanner schema table wise or globally</li>
-                    <li>- Click on download session file button on the top right corner</li>
-                    <li>- Save the generated session file with all the changes in local machine</li>
+                    <li> Save all the changes done in spanner schema table wise or globally</li>
+                    <li> Click on download session file button on the top right corner</li>
+                    <li> Save the generated session file with all the changes in local machine</li>
                 </ul>
                 <br>
                 <img class="instructions-img-width" src='userManualImages/downloadSessionFile.png'>
diff --git a/frontend/js/pages/SchemaConversionScreen/SchemaConversionScreen.page.js b/frontend/js/pages/SchemaConversionScreen/SchemaConversionScreen.page.js
index 514e82d..7c1be3c 100644
--- a/frontend/js/pages/SchemaConversionScreen/SchemaConversionScreen.page.js
+++ b/frontend/js/pages/SchemaConversionScreen/SchemaConversionScreen.page.js
@@ -94,7 +94,7 @@ class SchemaConversionScreen extends HTMLElement {
     <div class="summary-main-content" id='schema-screen-content'>
       <div id="snackbar"></div>
       <div>
-        <h4 class="report-header">Recommended Schema Conversion Report
+        <h4 class="report-header">Schema Conversion Report
             <hb-site-button buttonid="download-schema" classname="download-button" 
                 buttonaction="downloadSession" text="Download Session File"></hb-site-button>
         </h4>
diff --git a/frontend/js/services/Action.service.js b/frontend/js/services/Action.service.js
index 43fd193..9b9e129 100644
--- a/frontend/js/services/Action.service.js
+++ b/frontend/js/services/Action.service.js
@@ -343,7 +343,8 @@ const Actions = (() => {
         fetchIndexFormValues: async(tableIndex, tableName, name, uniqueness) => {
             Actions.showSpinner()
             if (keysList.length === 0) {
-                showSnackbar("Please select atleast one key to create a new index", " redBg");
+                Actions.hideSpinner()
+                showSnackbar("Please select atleast one key column to create a new index", " redBg");
                 return;
             }
             let newIndex = {};
diff --git a/mysql/data.go b/mysql/data.go
index c051fa8..2990e3a 100644
--- a/mysql/data.go
+++ b/mysql/data.go
@@ -77,7 +77,7 @@ func ConvertData(conv *internal.Conv, srcTable string, srcCols []string, srcSche
 		if spColDef.T.IsArray {
 			x, err = convArray(spColDef.T, srcColDef.Type.Name, vals[i])
 		} else {
-			x, err = convScalar(spColDef.T, srcColDef.Type.Name, conv.TimezoneOffset, vals[i])
+			x, err = convScalar(conv, spColDef.T, srcColDef.Type.Name, conv.TimezoneOffset, vals[i])
 		}
 		if err != nil {
 			return "", []string{}, []interface{}{}, err
@@ -98,7 +98,7 @@ func ConvertData(conv *internal.Conv, srcTable string, srcCols []string, srcSche
 // appropriate Spanner value. It is the caller's responsibility to
 // detect and handle NULL values: convScalar will return error if a
 // NULL value is passed.
-func convScalar(spannerType ddl.Type, srcTypeName string, TimezoneOffset string, val string) (interface{}, error) {
+func convScalar(conv *internal.Conv, spannerType ddl.Type, srcTypeName string, TimezoneOffset string, val string) (interface{}, error) {
 	// Whitespace within the val string is considered part of the data value.
 	// Note that many of the underlying conversions functions we use (like
 	// strconv.ParseFloat and strconv.ParseInt) return "invalid syntax"
@@ -106,7 +106,7 @@ func convScalar(spannerType ddl.Type, srcTypeName string, TimezoneOffset string,
 	// We do not expect mysqldump to generate such output.
 	switch spannerType.Name {
 	case ddl.Bool:
-		return convBool(val)
+		return convBool(conv, val)
 	case ddl.Bytes:
 		return convBytes(val)
 	case ddl.Date:
@@ -126,9 +126,21 @@ func convScalar(spannerType ddl.Type, srcTypeName string, TimezoneOffset string,
 	}
 }
 
-func convBool(val string) (bool, error) {
+func convBool(conv *internal.Conv, val string) (bool, error) {
 	b, err := strconv.ParseBool(val)
 	if err != nil {
+		// MySQL uses TINYINT(1) to implement BOOL/BOOLEAN, and does not
+		// enforce/validate boolean values i.e. any value that can be stored
+		// in a TINYINT (-128 to 127) can be stored in BOOL/BOOLEAN.
+		// If ParseBool(val) fails, this is very likely the cause.
+		// To handle this, re-parse as INT64 and treat as true if value is non-zero.
+		// Note: if ParseBool(val) fails, then val is probably a non-zero number.
+		i, err2 := convInt64(val)
+		if err2 == nil && i >= -128 && i <= 127 {
+			b = i != 0
+			conv.Unexpected(fmt.Sprintf("Expected boolean value, but found integer value %v; mapping it to %v\n", val, b))
+			return b, err2
+		}
 		return b, fmt.Errorf("can't convert to bool: %w", err)
 	}
 	return b, err
diff --git a/mysql/mysqldump.go b/mysql/mysqldump.go
index 60cb17f..bde1445 100644
--- a/mysql/mysqldump.go
+++ b/mysql/mysqldump.go
@@ -25,6 +25,8 @@ import (
 	"github.com/cloudspannerecosystem/harbourbridge/schema"
 	"github.com/pingcap/parser"
 	"github.com/pingcap/parser/ast"
+	"github.com/pingcap/parser/opcode"
+	"github.com/pingcap/tidb/types"
 	driver "github.com/pingcap/tidb/types/parser_driver"
 )
 
@@ -726,6 +728,19 @@ func getVals(row []ast.ExprNode) ([]string, error) {
 		switch valueNode := item.(type) {
 		case *driver.ValueExpr:
 			values = append(values, fmt.Sprintf("%v", valueNode.GetValue()))
+		case *ast.UnaryOperationExpr:
+			if valueNode.Op != opcode.Minus {
+				return nil, fmt.Errorf("unexpected UnaryOperationExpr node with opcode %v", valueNode.Op)
+			}
+			valExpr, ok := valueNode.V.(*driver.ValueExpr)
+			if !ok {
+				return nil, fmt.Errorf("unexpected UnaryOperationExpr node with value type %T", valueNode.V)
+			}
+			value, err := getNegativeUnaryVals(valExpr)
+			if err != nil {
+				return nil, fmt.Errorf("unexpected UnaryOperationExpr node with value %v", valExpr.GetValue())
+			}
+			values = append(values, value)
 		default:
 			return nil, fmt.Errorf("unexpected value node %T", valueNode)
 		}
@@ -733,6 +748,21 @@ func getVals(row []ast.ExprNode) ([]string, error) {
 	return values, nil
 }
 
+func getNegativeUnaryVals(valExpr *driver.ValueExpr) (string, error) {
+	switch val := valExpr.GetValue().(type) {
+	case int64:
+		return fmt.Sprintf("%v", -1*val), nil
+	case *types.MyDecimal:
+		floatVal, err := val.ToFloat64()
+		if err != nil {
+			return "", fmt.Errorf("unexpected UnaryOperationExpr with value %v", val)
+		}
+		return fmt.Sprintf("%v", -1*floatVal), nil
+	default:
+		return "", fmt.Errorf("unexpected UnaryOperationExpr value with type %T", val)
+	}
+}
+
 func getTableNameInsert(stmt *ast.TableRefsClause) (string, error) {
 	if stmt.TableRefs == nil {
 		return "", fmt.Errorf("can't build table name as tablerefs is empty")
diff --git a/web/README.md b/web/README.md
index 4f44fbd..e9f66df 100644
--- a/web/README.md
+++ b/web/README.md
@@ -26,9 +26,9 @@ You can go to `<base-url>/` to visit the UI of the tool.
 
     eg: localhost:8080/
 
-For more details on how to use the UI, you can visit: `<base-url>/userManual.html`.
+For more details on how to use the UI, you can visit: `<base-url>/#/instructions`.
 
-    eg: localhost:8080/userManual.html
+    eg: localhost:8080/#/instructions
 
 <ins>**Note:**</ins>
 


Test patch:
diff --git a/mysql/data_test.go b/mysql/data_test.go
index 10f52e4..ea8e7ec 100644
--- a/mysql/data_test.go
+++ b/mysql/data_test.go
@@ -74,7 +74,13 @@ func TestConvertData(t *testing.T) {
 		in    string      // Input value for conversion.
 		e     interface{} // Expected result.
 	}{
-		{"bool", ddl.Type{Name: ddl.Bool}, "", "1", true},
+		{"bool 0", ddl.Type{Name: ddl.Bool}, "", "0", false},
+		{"bool 1", ddl.Type{Name: ddl.Bool}, "", "1", true},
+		{"bool true", ddl.Type{Name: ddl.Bool}, "", "true", true},
+		{"bool false", ddl.Type{Name: ddl.Bool}, "", "false", false},
+		{"bool 5", ddl.Type{Name: ddl.Bool}, "", "5", true},
+		{"bool -128", ddl.Type{Name: ddl.Bool}, "", "-128", true},
+		{"bool 127", ddl.Type{Name: ddl.Bool}, "", "127", true},
 		{"bytes", ddl.Type{Name: ddl.Bytes, Len: ddl.MaxLength}, "", string([]byte{137, 80}), []byte{0x89, 0x50}}, // need some other approach to testblob type
 		{"date", ddl.Type{Name: ddl.Date}, "", "2019-10-29", getDate("2019-10-29")},
 		{"float64", ddl.Type{Name: ddl.Float64}, "", "42.6", float64(42.6)},
@@ -227,6 +233,11 @@ func TestConvertError(t *testing.T) {
 			cols: []string{"a", "b", "c"},
 			vals: []string{"6", "6.6", "truee"},
 		},
+		{
+			name: "Error in bool 128",
+			cols: []string{"a", "b", "c"},
+			vals: []string{"6", "6.6", "128"},
+		},
 	}
 	tableName := "testtable"
 	spTable := ddl.CreateTable{
diff --git a/mysql/mysqldump_test.go b/mysql/mysqldump_test.go
index 1215b89..f8a365f 100644
--- a/mysql/mysqldump_test.go
+++ b/mysql/mysqldump_test.go
@@ -738,6 +738,15 @@ CREATE TABLE test (a text PRIMARY KEY, b text);`,
 			expectedData: []spannerData{
 				spannerData{table: "test", cols: []string{"id", "a", "b", "c", "d"}, vals: []interface{}{int64(1), int64(88), int64(44), int64(22), float64(444.9876)}}},
 		},
+		{
+			name: "Data conversion: negative values for smallint, mediumint, bigint, double",
+			input: `
+	CREATE TABLE test (id integer PRIMARY KEY, a smallint, b mediumint, c bigint, d double);
+	INSERT INTO test (id, a, b, c, d) VALUES (-1, -88, -44, -22, -444.9876);
+	`,
+			expectedData: []spannerData{
+				spannerData{table: "test", cols: []string{"id", "a", "b", "c", "d"}, vals: []interface{}{int64(-1), int64(-88), int64(-44), int64(-22), float64(-444.9876)}}},
+		},
 		// test with different timezone
 		{
 			name: "Data conversion:  text, timestamp, datetime, varchar",