From 4557315561f7ae7634a3b7824f54151c58499b47 Mon Sep 17 00:00:00 2001
From: Timofey Koolin <rekby@yandex-team.com>
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 <rekby@yandex-team.com>
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 <rekby@yandex-team.com>
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 <rekby@yandex-team.com>
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 <rekby@yandex-team.com>
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 <rekby@yandex-team.com>
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 <rekby@yandex-team.com>
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 <rekby@yandex-team.com>
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 <rekby@yandex-team.com>
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 <rekby@yandex-team.com>
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 <rekby@yandex-team.com>
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 <rekby@yandex-team.com>
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 <rekby@yandex-team.com>
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
}