From 4557315561f7ae7634a3b7824f54151c58499b47 Mon Sep 17 00:00:00 2001 From: Timofey Koolin Date: Thu, 17 Oct 2024 11:51:34 +0300 Subject: [PATCH 01/13] add err ErrIssue1501BadUUID --- internal/value/errors.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/value/errors.go b/internal/value/errors.go index 02843e811..9e4358b66 100644 --- a/internal/value/errors.go +++ b/internal/value/errors.go @@ -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 ) From 4f3db9ee52d92da7dee1c536e077e233000c7083 Mon Sep 17 00:00:00 2001 From: Timofey Koolin Date: Thu, 17 Oct 2024 14:51:01 +0300 Subject: [PATCH 02/13] add deprecated comments --- internal/params/parameters.go | 8 --- internal/table/scanner/scanner.go | 5 +- internal/value/value.go | 16 ++---- table/types/value.go | 6 -- tests/integration/query_regression_test.go | 57 ------------------- tests/integration/table_regression_test.go | 66 ---------------------- 6 files changed, 7 insertions(+), 151 deletions(-) diff --git a/internal/params/parameters.go b/internal/params/parameters.go index bea6bbf62..0dc26a0fe 100644 --- a/internal/params/parameters.go +++ b/internal/params/parameters.go @@ -297,14 +297,6 @@ func (p *Parameter) YSON(v []byte) Builder { return p.parent } -// UUID has data corruption bug and will be removed in next version. -// -// Deprecated: Use UUIDTyped (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) -} - // UUIDWithIssue1501Value is field serializer for save data with format bug. // For any new code use UUIDTyped // https://github.com/ydb-platform/ydb-go-sdk/issues/1501 diff --git a/internal/table/scanner/scanner.go b/internal/table/scanner/scanner.go index 3e16a4348..f498ace6b 100644 --- a/internal/table/scanner/scanner.go +++ b/internal/table/scanner/scanner.go @@ -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() + _ = s.errorf(0, "ydb: failed to scan uuid: %w", value.ErrIssue1501BadUUID) } return s.uint128() } @@ -775,6 +775,7 @@ func (s *valueScanner) setString(dst *string) { switch t := s.stack.current().t.GetTypeId(); t { case Ydb.Type_UUID: s.setUUIDStringWith1501Issue((*types.UUIDStringWithIssue1501Type)(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: @@ -797,7 +798,7 @@ func (s *valueScanner) setUUIDStringWith1501Issue(dst *types.UUIDStringWithIssue func (s *valueScanner) setByte(dst *[]byte) { switch t := s.stack.current().t.GetTypeId(); t { case Ydb.Type_UUID: - s.setUUIDWithIssue1501Byte((*value.UUIDIssue1501BytesSliceWrapper)(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: diff --git a/internal/value/value.go b/internal/value/value.go index 50552160f..dd6d8ea31 100644 --- a/internal/value/value.go +++ b/internal/value/value.go @@ -2108,27 +2108,19 @@ 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 *UUIDIssue1501StringWrapper: bytes := uuidReorderBytesForReadWithBug(v.value) *vv = UUIDIssue1501StringWrapper(bytes[:]) return nil case *[]byte: - bytes := uuidReorderBytesForReadWithBug(v.value) - *vv = bytes[:] - - return nil + return ErrIssue1501BadUUID case *UUIDIssue1501BytesSliceWrapper: bytes := uuidReorderBytesForReadWithBug(v.value) *vv = bytes[:] return nil case *[16]byte: - *vv = uuidReorderBytesForReadWithBug(v.value) - - return nil + return ErrIssue1501BadUUID case *UUIDIssue1501FixedBytesWrapper: *vv = uuidReorderBytesForReadWithBug(v.value) @@ -2150,7 +2142,7 @@ func (v *uuidValue) Yql() string { buffer.WriteString(v.Type().Yql()) buffer.WriteByte('(') buffer.WriteByte('"') - buffer.WriteString(uuid.UUID(v.value).String()) + buffer.WriteString(v.value.String()) buffer.WriteByte('"') buffer.WriteByte(')') diff --git a/table/types/value.go b/table/types/value.go index 03d2d78a4..eb24379c9 100644 --- a/table/types/value.go +++ b/table/types/value.go @@ -156,12 +156,6 @@ 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 UUIDTypedValue (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) } - // UUIDBytesWithIssue1501Type is type wrapper for scan expected values for values stored with bug // https://github.com/ydb-platform/ydb-go-sdk/issues/1501 type UUIDBytesWithIssue1501Type = value.UUIDIssue1501FixedBytesWrapper diff --git a/tests/integration/query_regression_test.go b/tests/integration/query_regression_test.go index f7b36778a..5e4588221 100644 --- a/tests/integration/query_regression_test.go +++ b/tests/integration/query_regression_test.go @@ -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 ( @@ -190,35 +162,6 @@ SELECT CAST($val AS UUID)`, 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 ( diff --git a/tests/integration/table_regression_test.go b/tests/integration/table_regression_test.go index 55dc0d24b..85d93d713 100644 --- a/tests/integration/table_regression_test.go +++ b/tests/integration/table_regression_test.go @@ -120,37 +120,6 @@ func TestUUIDSerializationTableServiceServiceIssue1501(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) - - var idFromDB string - err := db.Table().DoTx(ctx, func(ctx context.Context, tx table.TransactionActor) error { - res, err := tx.Execute(ctx, ` -DECLARE $val AS UUID; - -SELECT CAST($val AS Utf8) -`, table.NewQueryParameters(table.ValueParam("$val", types.UUIDValue(id)))) - if err != nil { - return err - } - res.NextResultSet(ctx) - res.NextRow() - - err = res.Scan(&idFromDB) - return err - }) - require.NoError(t, err) - require.Equal(t, expectedResultWithBug, idFromDB) - }) 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 ( @@ -438,41 +407,6 @@ SELECT CAST($val AS UUID) resultBytes := []byte(resultFromDb) require.Equal(t, expectedResultWithBug, resultBytes) }) - 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) - - var idFromDB uuid.UUID - err := db.Table().DoTx(ctx, func(ctx context.Context, tx table.TransactionActor) error { - res, err := tx.Execute(ctx, ` -DECLARE $val AS UUID; - -SELECT $val -`, table.NewQueryParameters(table.ValueParam("$val", types.UUIDValue(id)))) - if err != nil { - return err - } - res.NextResultSet(ctx) - res.NextRow() - - var resBytes [16]byte - err = res.Scan(&resBytes) - if err != nil { - return err - } - idFromDB = resBytes - return nil - }) - require.NoError(t, err) - require.Equal(t, id, idFromDB) - }) 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 ( From b9900d26786b619d481f0a6b6f9aa4496ad96119 Mon Sep 17 00:00:00 2001 From: Timofey Koolin Date: Thu, 17 Oct 2024 20:52:15 +0300 Subject: [PATCH 03/13] remove some code --- internal/bind/params.go | 4 ++-- tests/integration/query_regression_test.go | 11 ++--------- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/internal/bind/params.go b/internal/bind/params.go index 9d9ee0643..69a6602ae 100644 --- a/internal/bind/params.go +++ b/internal/bind/params.go @@ -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 + 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), nil case *types.UUIDBytesWithIssue1501Type: diff --git a/tests/integration/query_regression_test.go b/tests/integration/query_regression_test.go index 5e4588221..cf507e090 100644 --- a/tests/integration/query_regression_test.go +++ b/tests/integration/query_regression_test.go @@ -57,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; @@ -72,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.Error(t, err) }) 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 @@ -115,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; @@ -130,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.Error(t, err) }) 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 From 8cf442abcae6b6fa2451ce28a139e9b3c3fdf049 Mon Sep 17 00:00:00 2001 From: Timofey Koolin Date: Thu, 17 Oct 2024 21:34:34 +0300 Subject: [PATCH 04/13] some fixes --- table/types/errors.go | 5 ++++ .../database_sql_regression_test.go | 25 +++---------------- tests/integration/query_regression_test.go | 4 +-- tests/integration/table_regression_test.go | 6 +---- 4 files changed, 11 insertions(+), 29 deletions(-) create mode 100644 table/types/errors.go diff --git a/table/types/errors.go b/table/types/errors.go new file mode 100644 index 000000000..ad29ed3d5 --- /dev/null +++ b/table/types/errors.go @@ -0,0 +1,5 @@ +package types + +import "github.com/ydb-platform/ydb-go-sdk/v3/internal/value" + +var ErrIssue1501BadUUID = value.ErrIssue1501BadUUID diff --git a/tests/integration/database_sql_regression_test.go b/tests/integration/database_sql_regression_test.go index 97d471d01..80ec77c69 100644 --- a/tests/integration/database_sql_regression_test.go +++ b/tests/integration/database_sql_regression_test.go @@ -182,7 +182,6 @@ 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; @@ -190,13 +189,7 @@ 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 @@ -231,7 +224,6 @@ SELECT CAST($val AS Utf8)`, ) idString := "6E73B41C-4EDE-4D08-9CFB-B7462D9E498B" - expectedResultWithBug := "8b499e2d-46b7-fb9c-4d08-4ede6e73b41c" row := db.QueryRow(` DECLARE $val AS Text; @@ -244,10 +236,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 @@ -315,15 +304,7 @@ SELECT $val`, sql.Named("val", idParam), ) - require.NoError(t, row.Err()) - - var resBytes [16]byte - err := row.Scan(&resBytes) - require.NoError(t, err) - - resUUID := uuid.UUID(resBytes) - - require.Equal(t, id, resUUID) + 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 diff --git a/tests/integration/query_regression_test.go b/tests/integration/query_regression_test.go index 342d4e145..2eda59d4e 100644 --- a/tests/integration/query_regression_test.go +++ b/tests/integration/query_regression_test.go @@ -71,7 +71,7 @@ SELECT CAST($val AS UUID)`, var res [16]byte err = row.Scan(&res) - require.Error(t, err) + 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 @@ -125,7 +125,7 @@ SELECT CAST($val AS UUID)`, var res string err = row.Scan(&res) - require.Error(t, err) + 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 diff --git a/tests/integration/table_regression_test.go b/tests/integration/table_regression_test.go index 85d93d713..d5e1b06d9 100644 --- a/tests/integration/table_regression_test.go +++ b/tests/integration/table_regression_test.go @@ -160,8 +160,6 @@ SELECT CAST($val AS Utf8) ) idString := "6E73B41C-4EDE-4D08-9CFB-B7462D9E498B" - expectedResultWithBug := "8b499e2d-46b7-fb9c-4d08-4ede6e73b41c" - var resultFromDb uuid.UUID err := db.Table().DoTx(ctx, func(ctx context.Context, tx table.TransactionActor) error { res, err := tx.Execute(ctx, ` DECLARE $val AS Text; @@ -181,12 +179,10 @@ SELECT CAST($val AS UUID) return err } - resultFromDb = resBytes return nil }) - require.NoError(t, err) - require.Equal(t, expectedResultWithBug, resultFromDb.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 From 48e56041848719648ad74fa6bd4b3855af3a62fe Mon Sep 17 00:00:00 2001 From: Timofey Koolin Date: Thu, 17 Oct 2024 22:17:38 +0300 Subject: [PATCH 05/13] fix tests --- tests/integration/database_sql_regression_test.go | 2 +- tests/integration/table_regression_test.go | 9 ++------- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/tests/integration/database_sql_regression_test.go b/tests/integration/database_sql_regression_test.go index 8b54ac12d..49b16bfc6 100644 --- a/tests/integration/database_sql_regression_test.go +++ b/tests/integration/database_sql_regression_test.go @@ -236,7 +236,7 @@ SELECT CAST($val AS UUID)`, var res [16]byte err := row.Scan(&res) - require.ErrorIs(t, err, types.ErrIssue1501BadUUID) + require.Error(t, err) }) 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 diff --git a/tests/integration/table_regression_test.go b/tests/integration/table_regression_test.go index 2bc59d4fd..731a39430 100644 --- a/tests/integration/table_regression_test.go +++ b/tests/integration/table_regression_test.go @@ -267,7 +267,6 @@ SELECT CAST($val AS UUID) ) idString := "6E73B41C-4EDE-4D08-9CFB-B7462D9E498B" - expectedResultWithBug := "8b499e2d-46b7-fb9c-4d08-4ede6e73b41c" var resultFromDb customTestUnmarshalUUIDWithForceWrapper err := db.Table().DoTx(ctx, func(ctx context.Context, tx table.TransactionActor) error { res, err := tx.Execute(ctx, ` @@ -290,8 +289,7 @@ SELECT CAST($val AS UUID) return nil }) - require.NoError(t, err) - require.Equal(t, expectedResultWithBug, resultFromDb.String()) + require.ErrorIs(t, err, types.ErrIssue1501BadUUID) }) t.Run("old-unmarshal-with-bytes-typed", func(t *testing.T) { // test old behavior - for test way of safe work with data, written with bagged API version @@ -337,7 +335,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} var resultFromDb string err := db.Table().DoTx(ctx, func(ctx context.Context, tx table.TransactionActor) error { res, err := tx.Execute(ctx, ` @@ -360,9 +357,7 @@ SELECT CAST($val AS UUID) return nil }) - require.NoError(t, err) - resultBytes := []byte(resultFromDb) - require.Equal(t, expectedResultWithBug, resultBytes) + 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 From 299d96371b2aac1936134a47e0fe8c87479de0f1 Mon Sep 17 00:00:00 2001 From: Timofey Koolin Date: Thu, 17 Oct 2024 22:23:22 +0300 Subject: [PATCH 06/13] comment uuid in raw --- internal/scanner/scanner.go | 2 +- tests/integration/table_regression_test.go | 48 ---------------------- 2 files changed, 1 insertion(+), 49 deletions(-) diff --git a/internal/scanner/scanner.go b/internal/scanner/scanner.go index 1cba2f419..c4e6a08ca 100644 --- a/internal/scanner/scanner.go +++ b/internal/scanner/scanner.go @@ -38,8 +38,8 @@ type RawValue interface { UTF8() (v string) YSON() (v []byte) JSON() (v []byte) - UUID() (v [16]byte) UUIDTyped() (v uuid.UUID) + // UUID() (v [16]byte) removed for https://github.com/ydb-platform/ydb-go-sdk/issues/1501 UUIDWithIssue1501() (v [16]byte) JSONDocument() (v []byte) DyNumber() (v string) diff --git a/tests/integration/table_regression_test.go b/tests/integration/table_regression_test.go index 731a39430..7bb5bb919 100644 --- a/tests/integration/table_regression_test.go +++ b/tests/integration/table_regression_test.go @@ -223,41 +223,6 @@ SELECT CAST($val AS UUID) require.NoError(t, err) require.Equal(t, expectedResultWithBug, resultFromDb.String()) }) - t.Run("old-unmarshal-with-bytes", 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 := "8b499e2d-46b7-fb9c-4d08-4ede6e73b41c" - var resultFromDb customTestUnmarshalUUIDFromFixedBytes - err := db.Table().DoTx(ctx, func(ctx context.Context, tx table.TransactionActor) error { - res, err := tx.Execute(ctx, ` -DECLARE $val AS Text; - -SELECT CAST($val AS UUID) -`, table.NewQueryParameters(table.ValueParam("$val", types.TextValue(idString)))) - if err != nil { - return err - } - - res.NextResultSet(ctx) - res.NextRow() - - err = res.Scan(&resultFromDb) - if err != nil { - return err - } - - return nil - }) - - require.NoError(t, err) - require.Equal(t, expectedResultWithBug, resultFromDb.String()) - }) t.Run("old-unmarshal-with-bytes-force-wrapper", func(t *testing.T) { // test old behavior - for test way of safe work with data, written with bagged API version var ( @@ -525,19 +490,6 @@ SELECT $val }) } -type customTestUnmarshalUUIDFromFixedBytes string - -func (c *customTestUnmarshalUUIDFromFixedBytes) UnmarshalYDB(raw scanner.RawValue) error { - val := raw.UUID() - id := uuid.UUID(val) - *c = customTestUnmarshalUUIDFromFixedBytes(id.String()) - return raw.Err() -} - -func (c *customTestUnmarshalUUIDFromFixedBytes) String() string { - return string(*c) -} - type customTestUnmarshalUUIDWithForceWrapper string func (c *customTestUnmarshalUUIDWithForceWrapper) UnmarshalYDB(raw scanner.RawValue) error { From f1065eadced1b9686c4cf0d135b5f196ca99e0c4 Mon Sep 17 00:00:00 2001 From: Timofey Koolin Date: Thu, 17 Oct 2024 22:24:35 +0300 Subject: [PATCH 07/13] comment uuid in raw --- internal/table/scanner/scan_raw.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/internal/table/scanner/scan_raw.go b/internal/table/scanner/scan_raw.go index 6141e2edd..dabbf52b5 100644 --- a/internal/table/scanner/scan_raw.go +++ b/internal/table/scanner/scan_raw.go @@ -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 { From 3164744e75caf76454b9a99af349e7d3f40ab6fd Mon Sep 17 00:00:00 2001 From: Timofey Koolin Date: Thu, 17 Oct 2024 22:27:17 +0300 Subject: [PATCH 08/13] comment old public code instead of remove --- internal/params/parameters.go | 5 +++++ table/types/value.go | 3 +++ 2 files changed, 8 insertions(+) diff --git a/internal/params/parameters.go b/internal/params/parameters.go index 79c70e4cc..7c55adce4 100644 --- a/internal/params/parameters.go +++ b/internal/params/parameters.go @@ -297,6 +297,11 @@ func (p *Parameter) YSON(v []byte) Builder { return p.parent } +// 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 UUIDTyped // https://github.com/ydb-platform/ydb-go-sdk/issues/1501 diff --git a/table/types/value.go b/table/types/value.go index 8e8186e5b..39d6ab0d4 100644 --- a/table/types/value.go +++ b/table/types/value.go @@ -156,6 +156,9 @@ 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)) } +// 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 type UUIDBytesWithIssue1501Type = value.UUIDIssue1501FixedBytesWrapper From dccc591b69265c17a91883bb943c766b0c3f60f1 Mon Sep 17 00:00:00 2001 From: Timofey Koolin Date: Thu, 17 Oct 2024 22:38:01 +0300 Subject: [PATCH 09/13] add changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a3788196..acf5fc7cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,6 @@ +* 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). + * 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. * Added `db.Topic().DescribeTopicConsumer()` method for displaying consumer information From d2f4931e567fd579440912bacd0d4e8a5527d989 Mon Sep 17 00:00:00 2001 From: Timofey Koolin Date: Thu, 17 Oct 2024 23:29:30 +0300 Subject: [PATCH 10/13] fix param tests --- internal/bind/params_test.go | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/internal/bind/params_test.go b/internal/bind/params_test.go index 7b6dde8cf..87aeefd01 100644 --- a/internal/bind/params_test.go +++ b/internal/bind/params_test.go @@ -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}, From 3726c67d3a57c43b90bceb8164abda61ab566b14 Mon Sep 17 00:00:00 2001 From: Timofey Koolin Date: Tue, 22 Oct 2024 17:33:33 +0300 Subject: [PATCH 11/13] remove unused --- internal/table/scanner/scanner.go | 18 ------------------ table/types/value.go | 2 +- 2 files changed, 1 insertion(+), 19 deletions(-) diff --git a/internal/table/scanner/scanner.go b/internal/table/scanner/scanner.go index 64fb6dd6b..bb4f45baa 100644 --- a/internal/table/scanner/scanner.go +++ b/internal/table/scanner/scanner.go @@ -796,15 +796,6 @@ 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: @@ -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 { diff --git a/table/types/value.go b/table/types/value.go index 7af617291..f6e4fd5b8 100644 --- a/table/types/value.go +++ b/table/types/value.go @@ -157,7 +157,7 @@ func JSONValue(v string) Value { return value.JSONValue(v) } func JSONValueFromBytes(v []byte) Value { return value.JSONValue(xstring.FromBytes(v)) } // removed for https://github.com/ydb-platform/ydb-go-sdk/issues/1501 -//func UUIDValue(v [16]byte) Value { return UUIDWithIssue1501Value(v) } +// 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 From d98b602de7b37fcc4cd17a49e02e8e1c3302daa1 Mon Sep 17 00:00:00 2001 From: Timofey Koolin Date: Wed, 23 Oct 2024 13:37:03 +0300 Subject: [PATCH 12/13] fix scanner uuid --- internal/params/builder_test.go | 16 ---------------- internal/table/scanner/scanner.go | 7 +++++++ internal/table/scanner/scanner_data_test.go | 7 ++++--- internal/table/scanner/scanner_test.go | 17 ++++++++++------- internal/value/value.go | 15 +++++++++++---- 5 files changed, 32 insertions(+), 30 deletions(-) diff --git a/internal/params/builder_test.go b/internal/params/builder_test.go index 31f0569cc..20aa6b906 100644 --- a/internal/params/builder_test.go +++ b/internal/params/builder_test.go @@ -345,22 +345,6 @@ func TestBuilder(t *testing.T) { }, }, }, - { - method: "UUID", - args: []any{[...]byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}}, - - expected: expected{ - Type: &Ydb.Type{ - Type: &Ydb.Type_TypeId{TypeId: Ydb.Type_UUID}, - }, - Value: &Ydb.Value{ - Value: &Ydb.Value_Low_128{ - Low_128: 651345242494996240, - }, - High_128: 72623859790382856, - }, - }, - }, { method: "TzDatetime", args: []any{time.Unix(123456789, 456).UTC()}, diff --git a/internal/table/scanner/scanner.go b/internal/table/scanner/scanner.go index bb4f45baa..534f5b74a 100644 --- a/internal/table/scanner/scanner.go +++ b/internal/table/scanner/scanner.go @@ -1083,6 +1083,13 @@ func (s *valueScanner) scanOptional(v interface{}, defaultValueForOptional bool) val := value.NewUUIDIssue1501FixedBytesWrapper(src) *v = &val } + case **uuid.UUID: + if s.isNull() { + *v = nil + } else { + src := s.uuid() + *v = &src + } case **interface{}: if s.isNull() { *v = nil diff --git a/internal/table/scanner/scanner_data_test.go b/internal/table/scanner/scanner_data_test.go index f47fffbac..87c917ff8 100644 --- a/internal/table/scanner/scanner_data_test.go +++ b/internal/table/scanner/scanner_data_test.go @@ -5,6 +5,7 @@ import ( "strconv" "time" + "github.com/google/uuid" "github.com/ydb-platform/ydb-go-genproto/protos/Ydb" "github.com/ydb-platform/ydb-go-sdk/v3/table/result/indexed" @@ -63,7 +64,7 @@ var scannerData = []struct { name: "date", typeID: Ydb.Type_DATE, }}, - values: []indexed.RequiredOrOptional{new([16]byte), new(time.Time)}, + values: []indexed.RequiredOrOptional{new(uuid.UUID), new(time.Time)}, }, { name: "Scan JSON, DOUBLE", @@ -272,7 +273,7 @@ var scannerData = []struct { typeID: Ydb.Type_DOUBLE, optional: true, }}, - values: []indexed.RequiredOrOptional{new(*time.Duration), new(*[16]byte), new(*float64)}, + values: []indexed.RequiredOrOptional{new(*time.Duration), new(*uuid.UUID), new(*float64)}, }, { name: "Scan int64, date, string as ydb.Scanner", @@ -424,7 +425,7 @@ var scannerData = []struct { optional: true, nilValue: true, }}, - values: []indexed.RequiredOrOptional{new(*uint8), new(*[]byte), new(*time.Time), new(*[16]byte)}, + values: []indexed.RequiredOrOptional{new(*uint8), new(*[]byte), new(*time.Time), new(*uuid.UUID)}, }, { name: "Scan string as byte array.", diff --git a/internal/table/scanner/scanner_test.go b/internal/table/scanner/scanner_test.go index 44f09ff45..6bb714e43 100644 --- a/internal/table/scanner/scanner_test.go +++ b/internal/table/scanner/scanner_test.go @@ -8,6 +8,7 @@ import ( "testing" "time" + "github.com/google/uuid" "github.com/stretchr/testify/require" "github.com/ydb-platform/ydb-go-genproto/protos/Ydb" @@ -454,21 +455,23 @@ func valueFromPrimitiveTypeID(c *column, r xrand.Rand) (*Ydb.Value, interface{}) Value: &Ydb.Value_NullFlagValue{}, } if c.testDefault { - var dv [16]byte + var dv uuid.UUID return ydbval, &dv } - var dv *[16]byte + var dv *uuid.UUID return ydbval, &dv } - v := [16]byte{} - binary.BigEndian.PutUint64(v[0:8], uint64(rv)) - binary.BigEndian.PutUint64(v[8:16], uint64(rv)) + v := uuid.UUID{} + + binary.LittleEndian.PutUint64(v[0:8], uint64(rv)) + binary.LittleEndian.PutUint64(v[8:16], uint64(rv)) + low, high := value.UUIDToHiLoPair(v) ydbval := &Ydb.Value{ - High_128: binary.BigEndian.Uint64(v[0:8]), + High_128: high, Value: &Ydb.Value_Low_128{ - Low_128: binary.BigEndian.Uint64(v[8:16]), + Low_128: low, }, } if c.optional && !c.testDefault { diff --git a/internal/value/value.go b/internal/value/value.go index e9fc8d49f..6af9ca604 100644 --- a/internal/value/value.go +++ b/internal/value/value.go @@ -2173,20 +2173,27 @@ func (v *uuidValue) toYDB(a *allocator.Allocator) *Ydb.Value { return v.toYDBWithBug(a) } - var bytes [16]byte + var low, high uint64 if v != nil { - bytes = uuidDirectBytesToLe(v.value) + low, high = UUIDToHiLoPair(v.value) } vv := a.Low128() - vv.Low_128 = binary.LittleEndian.Uint64(bytes[0:8]) + vv.Low_128 = low vvv := a.Value() - vvv.High_128 = binary.LittleEndian.Uint64(bytes[8:16]) + vvv.High_128 = high vvv.Value = vv return vvv } +func UUIDToHiLoPair(id uuid.UUID) (low, high uint64) { + bytes := uuidDirectBytesToLe(id) + low = binary.LittleEndian.Uint64(bytes[0:8]) + high = binary.LittleEndian.Uint64(bytes[8:16]) + return low, high +} + func (v *uuidValue) toYDBWithBug(a *allocator.Allocator) *Ydb.Value { var bytes [16]byte if v != nil { From a33a5f2bc92e813bbd49d715e0cae7190cdc3ff0 Mon Sep 17 00:00:00 2001 From: Timofey Koolin Date: Wed, 23 Oct 2024 14:31:38 +0300 Subject: [PATCH 13/13] fix for linter --- internal/value/value.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/value/value.go b/internal/value/value.go index 6af9ca604..810140474 100644 --- a/internal/value/value.go +++ b/internal/value/value.go @@ -2191,6 +2191,7 @@ func UUIDToHiLoPair(id uuid.UUID) (low, high uint64) { bytes := uuidDirectBytesToLe(id) low = binary.LittleEndian.Uint64(bytes[0:8]) high = binary.LittleEndian.Uint64(bytes[8:16]) + return low, high }