Skip to content
This repository has been archived by the owner on Oct 17, 2023. It is now read-only.

Commit

Permalink
Handle certain column types in reader, not reader_test
Browse files Browse the repository at this point in the history
I.e. don't "make the tests pass" and do it properly.

In writing a comment on the PR I talked myself round to doing this. I'd 
written this:

> Does it make sense that we have all this [special handling in 
> `reader_test.go`][1] whereas Postgresql doesn't? I'm undecided. On 
> the one hand I think this shouldn't be required as reader.go should 
> do whatever is necessary, but on the other hand we don't necessarily
> specify how we are storing things at the Transporter level (i.e.
> compatibility layer between all adaptors, Golang native types). Maybe
> that should be in [`casifyValue` instead][2]? Could do with another
> pair of eyes, etc on this.

And then decided to handle things properly as much as possible. I.e. it 
does make sense for `casifyValue` to do this as we'd ultimately want 
the adaptor to pass that off to another adaptor, etc.

 I think what I have here is correct. Think.

There is one "hack" left in `reader_test.go` for the `colbinary` value 
of `0xDEADBEEF` and my reasons for that are explained in the comments. 
It doesn't stop binary data from working properly / as intended since 
the colblob test passes.

[1]: https://github.com/compose/transporter/blob/9d6d829ed2c2c71688c4d7e427147d83c228ac1b/adaptor/mysql/reader_test.go#L134-L167
[2]: https://github.com/compose/transporter/blob/9d6d829ed2c2c71688c4d7e427147d83c228ac1b/adaptor/mysql/reader.go#L201-L231
  • Loading branch information
atomicules committed May 10, 2022
1 parent 2982e76 commit c3fabd2
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 26 deletions.
30 changes: 30 additions & 0 deletions adaptor/mysql/reader.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package mysql

import (
"encoding/hex"
"database/sql"
"fmt"
"regexp"
Expand All @@ -13,6 +14,8 @@ import (
"github.com/compose/transporter/message"
"github.com/compose/transporter/message/data"
"github.com/compose/transporter/message/ops"
"github.com/twpayne/go-geom/encoding/wkbhex"
"github.com/twpayne/go-geom/encoding/wkt"
)

var (
Expand Down Expand Up @@ -204,6 +207,33 @@ func casifyValue(value string, valueType string) interface{} {
switch {
case value == "null":
return nil
// NOTE: No need to do this for true binary data, but _if_ we wanted a string
// representation (`deadbeef000000000000`) of binary data (`0xDEADBEEF`) then this
// would be what we'd do
//
//case valueType == "binary" || valueType == "blob":
// b := hex.EncodeToString([]byte(value))
// return b
case valueType == "bit":
bithexvalue := hex.EncodeToString([]byte(value))
bitintvalue, err := strconv.ParseInt(bithexvalue, 10, 64)
if err != nil {
fmt.Printf("\nBit (%v) parse error: %v\n\n", value, err)
}
bitvalue := strconv.FormatInt(bitintvalue, 2)
return bitvalue
case valueType == "geometry" || valueType == "point" || valueType == "linestring" || valueType == "polygon" || valueType == "multipoint" || valueType == "multilinestring" || valueType == "multipolygon" || valueType == "geometrycollection":
geomhexvalue := hex.EncodeToString([]byte(value))
// Strip SRID
geom, err := wkbhex.Decode(geomhexvalue[8:])
if err != nil {
fmt.Printf("\nSpatial hex (%v) parse error: %v\n\n", value, err)
}
wktGeom, err := wkt.Marshal(geom)
if err != nil {
fmt.Printf("\nSpatial (%v) parse error: %v\n\n", value, err)
}
return wktGeom
case valueType == "int" || valueType == "smallint" || valueType == "tinyint" || valueType == "mediumint" || valueType == "bigint":
// NOTE: No error handling here because copied Postgresql. Not really an excuse, but there you go
i, _ := strconv.ParseInt(value, 10, 64)
Expand Down
44 changes: 18 additions & 26 deletions adaptor/mysql/reader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,13 @@ package mysql
import (
_ "embed"
"encoding/hex"
//"encoding/json"
"fmt"
"strconv"
"strings"
"testing"
"time"

"github.com/compose/transporter/client"
"github.com/compose/transporter/message"
"github.com/twpayne/go-geom/encoding/wkbhex"
"github.com/twpayne/go-geom/encoding/wkt"
)

var (
Expand Down Expand Up @@ -131,34 +127,30 @@ func TestReadComplex(t *testing.T) {
"colpolygon": "POLYGON ((0 0, 10 0, 10 10, 0 10, 0 0), (5 5, 7 5, 7 7, 5 7, 5 5))",
"colgeometrycollection": "GEOMETRYCOLLECTION (POINT (1 1), LINESTRING (0 0, 1 1, 2 2, 3 3, 4 4))",
} {
// Some values need additional parsing.
// TODO: See what we can do to tidy things up here
switch {
case key == "colbinary":
// NOTE: This is a "hack" for testing purposes.
// True binary data (colblob) works fine and no additional parsing is required
// (i.e. nothing in `casifyValue` for it and the blob comparison works)
// When we insert the Golang value of 0xDEADBEEF into MySQL and just read it we
// get a weird string. I.e. the actual binary data. But I cannot for the life
// of me figure out how in Golang to convert 0xDEADBEEF to the same form. I.e.
// like the blobdata. So...
//
// In a mysql shell you can get a human readable form from:
//
// mysql> select hex(colbinary) from reader_complex_test_table limit 1;
// +----------------------+
// | hex(colbinary) |
// +----------------------+
// | DEADBEEF000000000000 |
// +----------------------+
//
// So that is what we do here just for the ease of testing
binvalue := hex.EncodeToString([]byte(msgs[i].Data().Get(key).(string)))
if binvalue != value {
t.Errorf("Expected %v of row to equal %v (%T), but was %v (%T)", key, value, value, binvalue, binvalue)
}
case key == "colbit":
// :puke
bithexvalue := hex.EncodeToString([]byte(msgs[i].Data().Get(key).(string)))
// NOTE: No error handling on the below since this is a test file
bitintvalue, _ := strconv.ParseInt(bithexvalue, 10, 64)
bitvalue := strconv.FormatInt(bitintvalue, 2)
if bitvalue != value {
t.Errorf("Expected %v of row to equal %v (%T), but was %v (%T)", key, value, value, bitvalue, bitvalue)
}
case key == "colpoint" || key == "collinestring" || key == "colpolygon" || key == "colgeometrycollection":
// There is no t.Debugf unfortunately so keeping below but commented out
//t.Logf("DEBUG: %v (%T)", msgs[i].Data().Get(key), msgs[i].Data().Get(key))
geomhexvalue := hex.EncodeToString([]byte(msgs[i].Data().Get(key).(string)))
// Strip SRID
// NOTE: No error handling on the below since this is a test file
geom, _ := wkbhex.Decode(geomhexvalue[8:])
wktGeom, _ := wkt.Marshal(geom)
if wktGeom != value {
t.Errorf("Expected %v of row to equal %v (%T), but was %v (%T)", key, value, value, wktGeom, wktGeom)
}
default:
if msgs[i].Data().Get(key) != value {
// Fatalf here hides other errors because it's a FailNow so use Error instead
Expand Down

0 comments on commit c3fabd2

Please sign in to comment.