Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UUID break old code #1523

Merged
merged 27 commits into from
Oct 23, 2024
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
4557315
add err ErrIssue1501BadUUID
rekby Oct 17, 2024
e7513f3
Merge branch 'uuid-fix' into uuid-fix-break
rekby Oct 17, 2024
4f3db9e
add deprecated comments
rekby Oct 17, 2024
a99a740
Merge branch 'uuid-fix' into uuid-fix-break
rekby Oct 17, 2024
b9900d2
remove some code
rekby Oct 17, 2024
1c7ba7d
Merge branch 'uuid-fix' into uuid-fix-break
rekby Oct 17, 2024
c006b32
Merge branch 'uuid-fix' into uuid-fix-break
rekby Oct 17, 2024
19c7ec8
Merge branch 'uuid-fix' into uuid-fix-break
rekby Oct 17, 2024
8cf442a
some fixes
rekby Oct 17, 2024
0b4572e
Merge branch 'uuid-fix' into uuid-fix-break
rekby Oct 17, 2024
48e5604
fix tests
rekby Oct 17, 2024
299d963
comment uuid in raw
rekby Oct 17, 2024
f1065ea
comment uuid in raw
rekby Oct 17, 2024
3164744
comment old public code instead of remove
rekby Oct 17, 2024
80b9f04
Merge branch 'uuid-fix' into uuid-fix-break
rekby Oct 17, 2024
dccc591
add changelog
rekby Oct 17, 2024
76edf5b
Merge branch 'uuid-fix' into uuid-fix-break
rekby Oct 17, 2024
e6c4b1c
Merge branch 'uuid-fix' into uuid-fix-break
rekby Oct 17, 2024
d2f4931
fix param tests
rekby Oct 17, 2024
0c2ae2c
Merge branch 'uuid-fix' into uuid-fix-break
rekby Oct 22, 2024
8b59152
Merge branch 'uuid-fix' into uuid-fix-break
rekby Oct 22, 2024
3726c67
remove unused
rekby Oct 22, 2024
2b063e4
Merge branch 'uuid-fix' into uuid-fix-break
rekby Oct 23, 2024
c286a45
Merge branch 'master' into uuid-fix-break
rekby Oct 23, 2024
d98b602
fix scanner uuid
rekby Oct 23, 2024
bce0cf8
Merge branch 'master' into uuid-fix-break
rekby Oct 23, 2024
a33a5f2
fix for linter
rekby Oct 23, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
* BREAK OLD STYLE WORK WITH UUID. See https://github.com/ydb-platform/ydb-go-sdk/issues/1501 for details.
At the version you must explicit choose way for work with uuid: old with bug or new (fixed).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
At the version you must explicit choose way for work with uuid: old with bug or new (fixed).
This version, you must explicit choose way to work with uuid: old, with bug, or new (fixed).


## v3.86.0
* Add workaround for bug in uuid send/receive from server. It is migration version. All native code and most database sql code worked with uuid continue to work.
Dedicated version for migrate code for workaround/fix uuid bug. See https://github.com/ydb-platform/ydb-go-sdk/issues/1501 for details.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Dedicated version for migrate code for workaround/fix uuid bug. See https://github.com/ydb-platform/ydb-go-sdk/issues/1501 for details.
Dedicated version to enable code migration from buggy way of sending UUID data to server to correct implementation. See https://github.com/ydb-platform/ydb-go-sdk/issues/1501 for details.


## v3.85.3
* Renamed `query.WithPoolID()` into `query.WithResourcePool()`
Expand Down
4 changes: 2 additions & 2 deletions internal/bind/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,9 @@ func toValue(v interface{}) (_ types.Value, err error) {

return types.ListValue(items...), nil
case [16]byte:
return types.UUIDValue(x), nil //nolint:staticcheck
return nil, xerrors.Wrap(value.ErrIssue1501BadUUID)
case *[16]byte:
return types.NullableUUIDValue(x), nil
return nil, xerrors.Wrap(value.ErrIssue1501BadUUID)
case types.UUIDBytesWithIssue1501Type:
return types.UUIDWithIssue1501Value(x.AsBytesArray()), nil
case *types.UUIDBytesWithIssue1501Type:
Expand Down
13 changes: 6 additions & 7 deletions internal/bind/params_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -279,21 +279,20 @@ func TestToValue(t *testing.T) {
dst: types.ListValue(types.TextValue("test")),
err: nil,
},

{
src: [16]byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16},
dst: types.UUIDValue([16]byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}), //nolint:staticcheck
err: nil,
dst: nil,
err: types.ErrIssue1501BadUUID,
},
{
src: func() *[16]byte { return nil }(),
dst: types.NullValue(types.TypeUUID),
err: nil,
dst: nil,
err: types.ErrIssue1501BadUUID,
},
{
src: func(v [16]byte) *[16]byte { return &v }([16]byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}),
dst: types.OptionalValue(types.UUIDValue([16]byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16})), //nolint:staticcheck,lll
err: nil,
dst: nil,
err: types.ErrIssue1501BadUUID,
},
{
src: uuid.UUID{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16},
Expand Down
11 changes: 4 additions & 7 deletions internal/params/parameters.go
Original file line number Diff line number Diff line change
Expand Up @@ -297,13 +297,10 @@ func (p *Parameter) YSON(v []byte) Builder {
return p.parent
}

// UUID has data corruption bug and will be removed in next version.
//
// Deprecated: Use Uuid (prefer) or UUIDWithIssue1501Value (for save old behavior) instead.
// https://github.com/ydb-platform/ydb-go-sdk/issues/1501
func (p *Parameter) UUID(v [16]byte) Builder {
return p.UUIDWithIssue1501Value(v)
}
// removed for https://github.com/ydb-platform/ydb-go-sdk/issues/1501
//func (p *Parameter) UUID(v [16]byte) Builder {
// return p.UUIDWithIssue1501Value(v)
//}

// UUIDWithIssue1501Value is field serializer for save data with format bug.
// For any new code use Uuid
Expand Down
7 changes: 1 addition & 6 deletions internal/scanner/scanner.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,7 @@ type RawValue interface {
UTF8() (v string)
YSON() (v []byte)
JSON() (v []byte)

// UUID has data corruption bug and will be removed in next version.
//
// Deprecated: Use UUIDTyped (prefer) or UUIDWithIssue1501 (for save old behavior) instead.
// https://github.com/ydb-platform/ydb-go-sdk/issues/1501
UUID() (v [16]byte)
// UUID() (v [16]byte) removed for https://github.com/ydb-platform/ydb-go-sdk/issues/1501
UUIDTyped() (v uuid.UUID)
UUIDWithIssue1501() (v [16]byte)
JSONDocument() (v []byte)
Expand Down
7 changes: 4 additions & 3 deletions internal/table/scanner/scan_raw.go
Original file line number Diff line number Diff line change
Expand Up @@ -263,9 +263,10 @@ func (s *rawConverter) JSONDocument() (v []byte) {
return xstring.ToBytes(s.text())
}

func (s *rawConverter) UUID() (v [16]byte) {
return s.uuidBytesWithIssue1501().AsBytesArray()
}
// removed for https://github.com/ydb-platform/ydb-go-sdk/issues/1501
//func (s *rawConverter) UUID() (v [16]byte) {
// return s.uuidBytesWithIssue1501().AsBytesArray()
//}

func (s *rawConverter) UUIDWithIssue1501() (v [16]byte) {
if s.Err() != nil {
Expand Down
24 changes: 3 additions & 21 deletions internal/table/scanner/scanner.go
Original file line number Diff line number Diff line change
Expand Up @@ -694,7 +694,7 @@ func (s *valueScanner) low128() (v uint64) {

func (s *valueScanner) uint128() (v [16]byte) {
if s.stack.current().t.GetTypeId() == Ydb.Type_UUID {
return s.uuidBytesWithIssue1501().AsBytesArray()
_ = s.errorf(0, "ydb: failed to scan uuid: %w", value.ErrIssue1501BadUUID)
}

c := s.stack.current()
Expand Down Expand Up @@ -786,7 +786,7 @@ func (s *valueScanner) setTime(dst *time.Time) {
func (s *valueScanner) setString(dst *string) {
switch t := s.stack.current().t.GetTypeId(); t {
case Ydb.Type_UUID:
s.setUUIDStringWith1501Issue(dst)
_ = s.errorf(0, "ydb: failed scan uuid: %w", value.ErrIssue1501BadUUID)
case Ydb.Type_UTF8, Ydb.Type_DYNUMBER, Ydb.Type_YSON, Ydb.Type_JSON, Ydb.Type_JSON_DOCUMENT:
*dst = s.text()
case Ydb.Type_STRING:
Expand All @@ -796,19 +796,10 @@ func (s *valueScanner) setString(dst *string) {
}
}

func (s *valueScanner) setUUIDStringWith1501Issue(dst *string) {
switch t := s.stack.current().t.GetTypeId(); t {
case Ydb.Type_UUID:
*dst = s.uuidBytesWithIssue1501().AsBrokenString()
default:
_ = s.errorf(0, "scan row failed: incorrect source types %s", t)
}
}

func (s *valueScanner) setByte(dst *[]byte) {
switch t := s.stack.current().t.GetTypeId(); t {
case Ydb.Type_UUID:
s.setUUIDWithIssue1501Byte(dst)
_ = s.errorf(0, "ydb: failed to scan uuid: %w", value.ErrIssue1501BadUUID)
case Ydb.Type_UTF8, Ydb.Type_DYNUMBER, Ydb.Type_YSON, Ydb.Type_JSON, Ydb.Type_JSON_DOCUMENT:
*dst = xstring.ToBytes(s.text())
case Ydb.Type_STRING:
Expand All @@ -818,15 +809,6 @@ func (s *valueScanner) setByte(dst *[]byte) {
}
}

func (s *valueScanner) setUUIDWithIssue1501Byte(dst *[]byte) {
switch t := s.stack.current().t.GetTypeId(); t {
case Ydb.Type_UUID:
*dst = s.uuidBytesWithIssue1501().AsBytesSlice()
default:
_ = s.errorf(0, "scan row failed: incorrect source type %s", t)
}
}

func (s *valueScanner) trySetByteArray(v interface{}, optional, def bool) bool {
rv := reflect.ValueOf(v)
if rv.Kind() == reflect.Ptr {
Expand Down
1 change: 1 addition & 0 deletions internal/value/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,5 @@ var (
ErrCannotCast = errors.New("cast failed")
errDestinationTypeIsNotAPointer = errors.New("destination type is not a pointer")
errNilDestination = errors.New("destination is nil")
ErrIssue1501BadUUID = errors.New("ydb: uuid storage format was broken in go SDK. Now it fixed. And you should select variant for work: typed uuid (good) or use old format with explicit wrapper for read old data") //nolint:lll
)
14 changes: 3 additions & 11 deletions internal/value/value.go
Original file line number Diff line number Diff line change
Expand Up @@ -2130,19 +2130,11 @@ type uuidValue struct {
func (v *uuidValue) castTo(dst interface{}) error {
switch vv := dst.(type) {
case *string:
bytes := uuidReorderBytesForReadWithBug(v.value)
*vv = string(bytes[:])

return nil
return ErrIssue1501BadUUID
case *[]byte:
bytes := uuidReorderBytesForReadWithBug(v.value)
*vv = bytes[:]

return nil
return ErrIssue1501BadUUID
case *[16]byte:
*vv = uuidReorderBytesForReadWithBug(v.value)

return nil
return ErrIssue1501BadUUID
case *UUIDIssue1501FixedBytesWrapper:
*vv = NewUUIDIssue1501FixedBytesWrapper(uuidReorderBytesForReadWithBug(v.value))

Expand Down
5 changes: 5 additions & 0 deletions table/types/errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package types

import "github.com/ydb-platform/ydb-go-sdk/v3/internal/value"

var ErrIssue1501BadUUID = value.ErrIssue1501BadUUID
7 changes: 2 additions & 5 deletions table/types/value.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,11 +156,8 @@ func JSONValue(v string) Value { return value.JSONValue(v) }
// (functional will be implements with go1.18 type lists)
func JSONValueFromBytes(v []byte) Value { return value.JSONValue(xstring.FromBytes(v)) }

// UUIDValue has data corruption bug and will be removed in next version.
//
// Deprecated: Use UuidValue (prefer) or UUIDWithIssue1501Value (for save old behavior) instead.
// https://github.com/ydb-platform/ydb-go-sdk/issues/1501
func UUIDValue(v [16]byte) Value { return UUIDWithIssue1501Value(v) }
// removed for https://github.com/ydb-platform/ydb-go-sdk/issues/1501
// func UUIDValue(v [16]byte) Value { return UUIDWithIssue1501Value(v) }

// UUIDBytesWithIssue1501Type is type wrapper for scan expected values for values stored with bug
// https://github.com/ydb-platform/ydb-go-sdk/issues/1501
Expand Down
15 changes: 2 additions & 13 deletions tests/integration/database_sql_regression_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,21 +182,14 @@ func TestUUIDSerializationDatabaseSQLIssue1501(t *testing.T) {
)

idString := "6E73B41C-4EDE-4D08-9CFB-B7462D9E498B"
expectedResultWithBug := "2d9e498b-b746-9cfb-084d-de4e1cb4736e"
id := [16]byte(uuid.MustParse(idString))
row := db.QueryRow(`
DECLARE $val AS UUID;

SELECT CAST($val AS Utf8)`, sql.Named("val", id),
)

require.NoError(t, row.Err())

var res string

err := row.Scan(&res)
require.NoError(t, err)
require.Equal(t, expectedResultWithBug, res)
require.ErrorIs(t, row.Err(), types.ErrIssue1501BadUUID)
})
t.Run("old-send-with-force-wrapper", func(t *testing.T) {
// test old behavior - for test way of safe work with data, written with bagged API version
Expand Down Expand Up @@ -333,11 +326,7 @@ SELECT $val`,
sql.Named("val", idParam),
)

require.NoError(t, row.Err())

var resBytes [16]byte
err := row.Scan(&resBytes)
require.Error(t, err)
require.ErrorIs(t, row.Err(), types.ErrIssue1501BadUUID)
})
t.Run("old-send-receive-with-force-wrapper", func(t *testing.T) {
// test old behavior - for test way of safe work with data, written with bagged API version
Expand Down
68 changes: 2 additions & 66 deletions tests/integration/query_regression_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,34 +20,6 @@ func TestUUIDSerializationQueryServiceIssue1501(t *testing.T) {
// https://github.com/ydb-platform/ydb-go-sdk/issues/1501
// test with special uuid - all bytes are different for check any byte swaps

t.Run("old-send", func(t *testing.T) {
// test old behavior - for test way of safe work with data, written with bagged API version
var (
scope = newScope(t)
ctx = scope.Ctx
db = scope.Driver()
)

idString := "6E73B41C-4EDE-4D08-9CFB-B7462D9E498B"
expectedResultWithBug := "2d9e498b-b746-9cfb-084d-de4e1cb4736e"
id := uuid.MustParse(idString)
row, err := db.Query().QueryRow(ctx, `
DECLARE $val AS UUID;

SELECT CAST($val AS Utf8)`,
query.WithIdempotent(),
query.WithParameters(ydb.ParamsBuilder().Param("$val").UUID(id).Build()),
query.WithTxControl(tx.SerializableReadWriteTxControl()),
)

require.NoError(t, err)

var res string

err = row.Scan(&res)
require.NoError(t, err)
require.Equal(t, expectedResultWithBug, res)
})
t.Run("old-send-with-force-wrapper", func(t *testing.T) {
// test old behavior - for test way of safe work with data, written with bagged API version
var (
Expand Down Expand Up @@ -85,7 +57,6 @@ SELECT CAST($val AS Utf8)`,
)

idString := "6E73B41C-4EDE-4D08-9CFB-B7462D9E498B"
expectedResultWithBug := "8b499e2d-46b7-fb9c-4d08-4ede6e73b41c"
row, err := db.Query().QueryRow(ctx, `
DECLARE $val AS Text;

Expand All @@ -100,10 +71,7 @@ SELECT CAST($val AS UUID)`,
var res [16]byte

err = row.Scan(&res)
require.NoError(t, err)

resUUID := uuid.UUID(res)
require.Equal(t, expectedResultWithBug, resUUID.String())
require.ErrorIs(t, err, types.ErrIssue1501BadUUID)
})
t.Run("old-receive-to-bytes-with-force-wrapper", func(t *testing.T) {
// test old behavior - for test way of safe work with data, written with bagged API version
Expand Down Expand Up @@ -143,7 +111,6 @@ SELECT CAST($val AS UUID)`,
)

idString := "6E73B41C-4EDE-4D08-9CFB-B7462D9E498B"
expectedResultWithBug := []byte{0x8b, 0x49, 0x9e, 0x2d, 0x46, 0xb7, 0xfb, 0x9c, 0x4d, 0x8, 0x4e, 0xde, 0x6e, 0x73, 0xb4, 0x1c}
row, err := db.Query().QueryRow(ctx, `
DECLARE $val AS Text;

Expand All @@ -158,9 +125,7 @@ SELECT CAST($val AS UUID)`,
var res string

err = row.Scan(&res)
require.NoError(t, err)

require.Equal(t, expectedResultWithBug, []byte(res))
require.ErrorIs(t, err, types.ErrIssue1501BadUUID)
})
t.Run("old-receive-to-string-with-force-wrapper", func(t *testing.T) {
// test old behavior - for test way of safe work with data, written with bagged API version
Expand Down Expand Up @@ -191,35 +156,6 @@ SELECT CAST($val AS UUID)`,
res := resFromDB.AsBrokenString()
require.Equal(t, expectedResultWithBug, []byte(res))
})
t.Run("old-send-receive", func(t *testing.T) {
// test old behavior - for test way of safe work with data, written with bagged API version
var (
scope = newScope(t)
ctx = scope.Ctx
db = scope.Driver()
)

idString := "6E73B41C-4EDE-4D08-9CFB-B7462D9E498B"
id := uuid.MustParse(idString)
row, err := db.Query().QueryRow(ctx, `
DECLARE $val AS UUID;

SELECT $val`,
query.WithIdempotent(),
query.WithParameters(ydb.ParamsBuilder().Param("$val").UUID(id).Build()),
query.WithTxControl(tx.SerializableReadWriteTxControl()),
)

require.NoError(t, err)

var resBytes [16]byte
err = row.Scan(&resBytes)
require.NoError(t, err)

resUUID := uuid.UUID(resBytes)

require.Equal(t, id, resUUID)
})
t.Run("old-send-receive-with-force-wrapper", func(t *testing.T) {
// test old behavior - for test way of safe work with data, written with bagged API version
var (
Expand Down
Loading
Loading