tweaked scan method to work with most sql drivers // Added Must

[juliankoehn]

Stripped the Scan method a 'lil bit. As i had problems to Scan with sqlx library. All tests pass.

=== RUN   TestScan
--- PASS: TestScan (0.00s)
=== RUN   TestValue
--- PASS: TestValue (0.00s)

UnmarshalText(), UnmarshalBinary()are no longer required for Scan.

TestSqlScanner() from ksuid_test.go is still implemented and PASS

--- PASS: TestSqlScanner (0.00s)
    --- PASS: TestSqlScanner/<nil> (0.00s)
    --- PASS: TestSqlScanner/string (0.00s)
    --- PASS: TestSqlScanner/[]uint8 (0.00s)

Same for TestSqlValuer() --- PASS: TestSqlValuer (0.00s)

[rbranson]

@achille-roussel going to leave this one to you :) LMK if you want anything from me on it.

[achille-roussel]

Thanks for the contribution!

At first sight, I don't think we can merge the code as-is, because it changes the behavior of Scan in a way that could profoundly break some applications as it changes the way string values are handled.

I feel like it should be the database's responsibility to ensure that if a column is expected to hold KSUIDs, it should enforce that values are either exactly 20 or 27 bytes, depending on how form the KSUIDs are stored in (binary or text).

Maybe I'm not understanding well the problem, would you happen to have a more detailed example to share?

[achille-roussel]

These recursive calls to Scan cause heap allocations when packing the string value into an interface{}, this is why I have the unexported scan methods to use internally.

[juliankoehn]

@achille-roussel thanks for your time to review my request!

I see your Point.

Suggested change

	return i.Scan(string(src))
	return i.UnmarshalText(src)

Using the UnmarshalText is better. Both functions (scan old, scan new) are now allocating the same. Beside that the new Scan is consuming 2,79% less memory + is slightly faster.

[achille-roussel]

Adding this function seems unrelated to the modification of Scan itself, maybe we can split it in a separate PR?