Azure-cosmos-dotnet-v3: Misleading parameter PartitionKey in methods ReadItemStreamAsync and ReadItemAsync?

[Postlagerkarte]

Hi there,

ReadItemStreamAsyncand ReadItemAsyncboth expect a parameter PartitionKey partitionKey.

It requires to provide the value of the partition key, not the name of the the partition key.

I find this confusing because e.g. on the storage explorer and data explorer the term partionkey refers to the name of the key and not the value.

Maybe the name of the parameter can be adjusted to reflect that it is the value that should be passed in.

What is more, why do I have to create a partionkey object in the first place? Wouldn't it be easier to pass a string as parameter for the partitionkeyvalue and create the object internally?

[j82w]

I agree the parameter name partitionKeyValue would be better. In the SDK it always uses partitionKeyPath in places where the path is actually needed, so anywhere it mentions PartitionKey it means the partition key value.

By having a PartitionKey object it can be optimized so the serialized version that will go into the header can be cached. So you can use the same PartitionKey object and avoid the overhead of the serialization.
PartitionKey supports string, bool, double, null, none, and the possibility to add new types in the future. If there was a method overload for each type it would cause ReadItemAsync to have 5 overloads. In the user studies it caused a lot of negative feedback. It made the documentation verbose, and caused developers to miss helpful overloads on other methods.

One solution that is possible is to add an implicit conversion for those types. That way developers can just put a string and c# will call the implicit conversion to create the partition key object.

[bartelink]

🙀 No implicit operators for such a scenario please!

[j82w]

What's the reasoning to avoid implicit operators?

[bartelink]

Just Explicit is better than Implicit rule in general. For important stuff like the partition key spec, Making Me Think a little bit is a good thing. i.e. being forced to write new PartionKeyValue(key) versus some magic doing that for me means (in this strawman example)
a) extra allocations
b) I dont recognize that this might be something I want to have a strong type for
c) I don't get to see that PartionKeyValue can also be constructed with an int

Implicit operators for me need to be a) really high traffic b) really improve readability for common cases. I think XLinq uses them and was a good design (perhaps someone who has dealt with feedback 2 years down the road might disagree)

[Postlagerkarte]

I understand what you are saying about the overload being an object instead of a primitive type and the reasoning behind it now. Thank you for explaining.

However does it apply to cases where you want to read by id? Or do you consider reading just by id - without partionkey - something that rarely happens?

Currently, if you want to read an item by id:

container.ReadItemAsync("42", PartitionKey.None) or container.ReadItemAsync("42", new PartitionKey("mykeyvalue"));

I would like to be able to read by id with a simple and short statement. EitherReadItemAsync("42") or ReadItemById("42")

[j82w]

I suggest you look at the Partition documentation to better understand how Cosmos DB uses the partition key. TLDR: The PartitionKey is used to determine what machine is storing the data while the id tells what row on that machine. This allows the horizontal scaling.

[Postlagerkarte]

I am talking about scenarios where no partion key is used. I am under the impression that PartitionKey is required for collections > 10GB but optional for below. You already provide PartitionKey.None which can be passed into the method in those scenarios.

[j82w]

In the v3 SDK it is required to always provide a partition key path. In v2 it is optional. The problem is that people where creating non-partitioned containers. Their apps and data would grow to the point that it would hit the cap. Once a container is created the partition key path can not be changed. This requires users to create a new partitioned container, and migrate all the data. This is a very painful process. To avoid this scenario all the new SDKs always make the container partitioned. The PartitionKey.None is used to allow access in the older non-partitioned containers.

If you are sure that your data will not need to scale you could define a new field like '/pk' as the partition key path. Then just have it always hard coded to string.Empty. That way you know that read will always be partition key value of string.empty. Then in the future if you find you do need to scale you can start populating that field with a real partition key value.

[bartelink]

Considering the use of CosmosDb without considering how the logical partitioning works and factors into whether stuff can be efficient and/or possible is a violation of "As Simple as Possible but No Simpler". While cross-partition stuff is possible and can be the right tool for the job, doing it without actively choosing that approach it the road to a slow expensive system that doesn't work with impossible to refactor code.