Merge pull request #1523 UUID break old code

rekby · web-flow · commit 5b9945c4e8a2 · 2024-10-23T15:06:06.000+03:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,8 +1,12 @@
+* 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).
+
 ## v3.86.1
 * Fixed scan to optional uuid
 
 ## 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.
 
 ## v3.85.3
 * Renamed `query.WithPoolID()` into `query.WithResourcePool()`
diff --git 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 //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:
diff --git 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},
diff --git 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/params/parameters.go b/internal/params/parameters.go
@@ -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
diff --git a/internal/scanner/scanner.go b/internal/scanner/scanner.go
@@ -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)
diff --git 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 {
diff --git 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().AsBytesArray()
+		_ = s.errorf(0, "ydb: failed to scan uuid: %w", value.ErrIssue1501BadUUID)
 	}
 
 	c := s.stack.current()
@@ -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:
@@ -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:
@@ -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/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
@@ -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/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
 )
diff --git a/internal/value/value.go b/internal/value/value.go
@@ -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))
 
@@ -2181,20 +2173,28 @@ 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 {
diff --git a/table/types/errors.go 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/table/types/value.go b/table/types/value.go
@@ -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
diff --git a/tests/integration/database_sql_regression_test.go b/tests/integration/database_sql_regression_test.go
@@ -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
@@ -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
diff --git a/tests/integration/query_regression_test.go b/tests/integration/query_regression_test.go
diff --git a/tests/integration/table_regression_test.go b/tests/integration/table_regression_test.go
