elastic/elasticsearch-rs

[ENHANCEMENT] index and create method argument from BulkOperation

Stargateur opened this issue · 2 comments

index is not logic, if I understand correctly, the purpose of index is "create or replace", so why it doesn't take a id argument like create, that is almost equivalent.

Possible fix:

  • index and create take an Into<Option<String>>
  • create should not take an id argument

Hi @Stargateur, the bulk operations model the bulk API:

  • An index operation optionally takes an _id; if the _id is specified then check if a document exists with that _id and if it does, overwrite it and increment the version number, otherwise create it. index operations are frequently used to index documents without _id, so BulkOperation::index(...) does not enforce a value to be passed, but if an _id needs to be specified, id(...) can be called on the returned BulkIndexOperation
  • A create operation requires an _id¹, so BulkOperation::create(...) requires it to be passed.

¹ this has recently changed with the introduction of data streams. A version of the client can be used with any version of Elasticsearch with the same major version though, so we probably still want to enforce an _id to be passed, because there are Elasticsearch 7.x versions where the _id is required for a create operation.

the doc clearly state in create:

_id (Optional, string) The document ID. If no ID is specified, a document ID is automatically generated.

it's only required by delete so I maintain what I said

¹ this has recently changed with the introduction of data streams. A version of the client can be used with any version of Elasticsearch with the same major version though, so we probably still want to enforce an _id to be passed, because there are Elasticsearch 7.x versions where the _id is required for a create operation.

I give up, elk have too much random change in major version I can't follow.

I mean, my first proposition is way better for the future, a good API is way better than follow a behavior in the 7.0 version of elk not documentation, you could still generate the id if the user provide none, but do what you want. At least index should take a Into<Option<String>> because it's clearly better