Ensure correct work UUID

[rekby]

The problem discussion started at ydb-platform/ydb#9799

Summary:
The SDK was send and receive UUID with bug: sdk byte order has differences to the server byte orders. It was not detected by unit tests and read-write tests (the SDK read same UUID, which was written).

Nobody saw the problem a lot of time, because UUID can't be stored on the server until last version. Now if the sdk send UUID to the server it and read it with YQL (cast to string) or other SDK (for example python) - readed UUID has differences from written.

Fix way in SDK:

1. Create dedicated minor migration version: 3.86.x, where old code work as is. In the version I add additional functions - for explicit read/write UUID with old behavior and for read/write with correct format. At the point need to explicit select way for every read/write UUID (old with bug or new).
2. Next dedicated minor version (3.87.x-3.88.x) break compatibility of old code as soon as possible: write code will be break at compile time and scan values code will be failed at runtime. All old code for work with UUID will be broken.

Migration for customers:
If you didn't use UUID before migration version: use good way functions. No need to do anything.

If you used the UUID and your code broken to compile or you have runtime error "ydb: uuid storage format was broken in go SDK..." you have to do change your code.

1. Change work with uuid with explicit old behavior (functions and type wrappers with ...Issue1501). See tests in migration version (add link).
2. Migrate your data and replace usage old-style work for new-style. For prevent mistakes and confusions when needs work with the data in other was than go sdk.

Migration example:

v3.86 - old-style code work with bug, no new code in sdk

Old code

		// 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)

upgrade ydb sdk to v3.87, old code still work

Same code

		// 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)

v3.87 upgrade code, without upgrade the sdk

Change function types.UUID to types.UUIDWithIssue1501Value for send value and use type types.UUIDBytesWithIssue1501Type for scan uuid value with old style.

After the changes code continue to work without change behavior

Update code to explicit work in old style

		// 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.UUIDWithIssue1501Value(id))))
			if err != nil {
				return err
			}
			res.NextResultSet(ctx)
			res.NextRow()

			var resBytes [16]byte
			var resWrapper types.UUIDBytesWithIssue1501Type
			err = res.Scan(&resWrapper)
			if err != nil {
				return err
			}
			resBytes = resWrapper.AsBytesArray()

			idFromDB = resBytes
			return nil
		})
		require.NoError(t, err)
		require.Equal(t, id, idFromDB)

Update SDK

Then you can upgrade the SDK without change the behavior and you have a time for data migration.

Data migration

Is is very dependency from project, purposre - rewrite uuid, stores in old style with bug to good uuid format. Which will be consistency with other SDKs and server YQL.

Change the code for work in new style - will work from v3.87 and later

During migration data for load/store good UUIDs and after migration use new-style code

New style code

		var (
			scope = newScope(t)
			ctx   = scope.Ctx
			db    = scope.Driver()
		)

		idString := "6E73B41C-4EDE-4D08-9CFB-B7462D9E498B"
		id := uuid.MustParse(idString)
		var resID 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))))

			res.NextResultSet(ctx)
			res.NextRow()
			err = res.ScanWithDefaults(&resID)
			if err != nil {
				return err
			}
			return nil
		})
		require.NoError(t, err)

		require.NoError(t, err)
		require.Equal(t, strings.ToUpper(idString), strings.ToUpper(resID.String()))

[hurd54]

it would be a good way to provide a set of examples illustrating the problem and the practice of correction.
step by step, specifying specific versions for migration.

[rekby]

@hurd54 done

[zveznicht]

1. What if data migration takes months, are we gonna be blocked to upgrade to newer version of sdk?
2. How sugar features like struct from result set binding are going to distinguish broken uuid from unboken ones? It is used in go-native sql libraries

[rekby]

1. Is is ok - you can use custom type as long as you need with special types similar to types.UUIDWithIssue1501Value(id)
2. What is go-native - database/sql?

Can you write small example of your code for check your case?

[zveznicht]

Is is ok - you can use custom type as long as you need with special types similar to types.UUIDWithIssue1501Value(id)

for example right now I have helper like this to map uuid into struct fields

type ScanUUID = *[16]byte

// And I can use it like this.
named.Required("event_id", ydbutil.ScanUUID(&r.EventID))

After upgrading I will have to rewrite all places with

var resWrapper types.UUIDBytesWithIssue1501Type
err = named.Required("event_id", &resWrapper)

r.EventID = resWrapper.AsBytesArray()

[zveznicht]

The closes example would be your sugar.UnmarshallRow (https://github.com/ydb-platform/ydb-go-sdk/blob/master/sugar/query.go#L17C6-L17C19)

If my struct use uuid.UUID field it won't be able to scan it as 'old uuid'

[zveznicht]

The closes example would be your sugar.UnmarshallRow (https://github.com/ydb-platform/ydb-go-sdk/blob/master/sugar/query.go#L17C6-L17C19)

If my struct use uuid.UUID field it won't be able to scan it as 'old uuid'

[rekby]

Do you use native sdk or database sql code?

Custom types in with native code can implement Scanner interface for override unmarshal behavior.

Example for uuid case:
https://github.com/ydb-platform/ydb-go-sdk/blob/9f8e82eeb817aea4b14eb920dfa81e59bbc312d7/tests/integration/table_regression_test.go#L493-L517

I didn't tested the behavior of custom type with database/sql, I can do it and guide for code fix code with minimal work.

If my struct use uuid.UUID field it won't be able to scan it as 'old uuid'.

if your code use uuid.UUID with database/sql - it should be break after upgrade #1515

for native sdk uuid.UUID can scan as 'new style' uuid only, but old versions of the native SDK can't scan uuid.UUID at all.

[rekby]

By plan code migration should be trivial (but if you have a lot of code - it can take fix in may places) and if you have some mistake (forget update some places) - new code stop to work (at compile time or runtime if it is impossible to runtime check) without data corruption.

[zveznicht]

for native sdk uuid.UUID can scan as 'new style' uuid only, but old versions of the native SDK can't scan uuid.UUID at all

they can, as bytes array. So code is clean, you scan your struct field by field with no boilerplate code.
something like this

res := SomeStruct{}
err := res.ScanNamed(
  named.Required("a", &res.A),
  named.Required("b", ydbutil.ScanUUID(&res.B)),
  named.Required("c", &res.C),
  named.Required("d", &res.D),
)

you don't need any helper variables to move fields from one representations to another one

Custom types in with native code can implement Scanner interface for override unmarshal behavior

we use native sdk. Unfortunately it doesn't seem to allow any custom unmarshallers

[rekby]

What about implement Scanner for ydbutil.ScanUUID type?

I can research it in details at start of next week.