-
Notifications
You must be signed in to change notification settings - Fork 604
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
UUID inconsistency in different SDKs #9799
Comments
Table: CREATE TABLE `/local/ydb_row_table` (
id Uint64,
val Uuid,
PRIMARY KEY (id)
) First we insert a row using the UI: UPSERT INTO `/local/ydb_row_table`
( `id`, `val` )
VALUES (1, Uuid("123e4567-e89b-12d3-a456-426614174000")); Let's now see how different SDKs handle this UUID: JavaWe can use one of three different UUID factory methods in Java List<Supplier<PrimitiveValue>> uuidGenerators = Arrays.asList(
() -> PrimitiveValue.newUuid("123e4567-e89b-12d3-a456-426614174000"), // [1]
() -> PrimitiveValue.newUuid(UUID.fromString("123e4567-e89b-12d3-a456-426614174000")), // [2]
() -> {
UUID uuid = UUID.fromString("123e4567-e89b-12d3-a456-426614174000");
return PrimitiveValue.newUuid(uuid.getMostSignificantBits(), uuid.getLeastSignificantBits()); // [3]
}
); 1 – from a string literal "SELECT * FROM `/local/ydb_row_table` val = $val" This way 1 and 2 works and 3 doesn't. In Java SDK UUID from a string literal and from java.util.UUID does some bit manipulations to make UUID according to the implementation from YQL that is used in YDB server. Manually creating UUID from MSB and LSB yields a different UUID. So on server (and consequentially using UI or CLI) this same literal will yield 16-byte array that is different from the one we create manually with MSB and LSB. PythonThe same as with Java, but Python already has an implementation of the UUID we use on server, so there is no need to wrap UUID as we do in Java. endpoint = "grpc://localhost:2136"
database = "/local"
with ydb.Driver(
endpoint=endpoint,
database=database,
) as driver:
driver.wait(timeout=5, fail_fast=True)
with ydb.QuerySessionPool(driver) as pool:
result_sets = pool.execute_with_retries(
"SELECT id, val FROM `/local/ydb_row_table` WHERE val = Uuid(\"123e4567-e89b-12d3-a456-426614174000\")"
)
print(result_sets) This will yield the correct UUID, since internally we use bytes_le constructor argument to build UUID object from byte array we receive from the server. GoIn Go SDK there is no wrapper for UUID and it is defined as follows // uuid.go
type UUID [16]byte Using var (
id uint64
val uuid.UUID
)
row, err := db.Query().QueryRow(ctx, "SELECT id, val FROM `/local/ydb_row_table` WHERE val = Uuid(\"123e4567-e89b-12d3-a456-426614174000\")")
if err = row.Scan(&id, (*[16]byte)(&val)); err != nil {
log.Printf("select failed: %v", err)
return
}
s, err := db.Table().CreateSession()
s.BulkUpsert()
uid, err := uuid.FromBytes(val[:])
log.Printf("id = %d, myStr = \"%s\"", id, uid.String()) And UUID value in this case will be Other languagesI was unable to find similar to Java or Python wrappers in SDKs for other languages, so I would assume that they either don't support UUID at all (seems like it is the case with Rust SDK) or do it the same way Go does, so the necessity of the bit manipulation is: |
The main reason for the problem in the Java SDK is the similarity of the internal representation of YDB's UUID and Java's UUID. They both have 128 bits and can be represented as two Java's long. At the same time, two methods of UUID creating use Java's UUID and Java's String and they are public. And one method uses two long, and this method is not intended for public use, it is used precisely to create a value from a protobuf. It can confuse users, so it is probably really better to hide that method from them (or deprecate for the beginning) |
With @SammyVimes, @alex268 and @dcherednik we discussed and came to decision. In every SDK and in YQL we will have two options
|
repro for future
|
Let's fix Go SDK by add new method for work with typed UUID and convert it internals to server representation UUID's and deprecate current UUID method/type. No need add FromBytes to other SDKs. It is a way for mistakes and confusing users: what method better? |
I found additional unexpected sorting behaviour: #10328 |
No description provided.
The text was updated successfully, but these errors were encountered: