NEST and Bulk operations of parital Updates (specifically UpdateMany)

Elastic Stack Elasticsearch
1

The current implementation of UpdateMany<T, TPartialDocument> operates on objects of type T instead of type TPartialDocument. This seems wrong to me, following the implementation of Update<T, TPartialDocument>, which operates on objects of TPartialDocument.

Am I misunderstanding how to use the UpdateMany to do a bulk partial update or is this just an API bug? I see it is possible to do bulk partial updates by creating my own BulkRequests, but I'd like to use the Fluent interface where possible.

Thanks!

1 Like
2

Hey @cscorley I do not think this is an API bug.

The update happens on documents of type T so UpdateMany iterates over a collection of T and uses the provided map selector to create instances of TPartialDocument which are than fed to the update descriptor as Doc(TPartialDocument doc) which sets the partial update to perform.

3

I spoke to soon that is how its suppose to operate but it doesn't, will create a PR for this.

Thank you for raising this!

4

Excellent, thank you!

5

I shall pivot once more, while looking at fixing this today I am convinced the current helper methods are correct.

/// <summary>
/// Updatemany, convenience method to pass many objects at once to do multiple updates.
/// </summary>
/// <param name="objects">the objects to update</param>
/// <param name="bulkUpdateSelector">An func called on each object to describe the individual update operation</param>
public BulkDescriptor UpdateMany<T, TPartialDocument>(IEnumerable<T> @objects, Func<BulkUpdateDescriptor<T, TPartialDocument>, T, IBulkUpdateOperation<T, TPartialDocument>> bulkUpdateSelector)
	where T : class
	where TPartialDocument : class =>
		Assign(a => @objects.ForEach(o => AddOperation(
			bulkUpdateSelector.InvokeOrDefault(
				new BulkUpdateDescriptor<T, TPartialDocument>().IdFrom(o)
				, o
			)
		)
	)
);

My initial response while looking at this was that o of type T as an argument should be of type TPartialDocument however it should not.

The helper makes no assumption on what kind of bulk update operation you want to compose inside bulkUpdateSelector this includes mapping from T to TPartialDocument if you want to specify the partial update via .Doc().

I hope this makes sense!

6

It still does not make sense. Why would I need to provide IEnumerable<T> (@objects) to accomplish a partial update with TPartialDocument in the bulkUpdateSelector? Building a TPartialDocument isn't an issue here, it's getting all those T just to satisfy the parameter requirements.

Does this mean I need to retrieve the original source documents of T myself? That seems to defeat the purpose of a quick partial update, and isn't something that's required in the non-Bulk operation so long as I know what document I'm updating:

var documentPath = new DocumentPath<Model>(Id.From(partialDocument));
var updateResponse = await Client.UpdateAsync<Model, PartialUpdate>(documentPath.Index(IndexName), x => x.Doc(partialDocument)));

Is this just to have a default operation for getting ids? I don't think it makes sense to create a bunch of T in order to extract an Id from -- I'd expect that Id to already be part of the TPartialDocument.

I think this method is just fine for doing T->TPartialDocument conversions, I'd just rather have a different method that only operated on TPartialDocument.

2 Likes
7

This topic was automatically closed 28 days after the last reply. New replies are no longer allowed.