Elasticsearch-net: Id property inference

[rikkit]

Referring to this page: http://nest.azurewebsites.net/nest/index-type-inference.html

You can control which propery holds the Id:

c# [ElasticType(IdProperty="CrazyGuid")] public class Car { public Guid CrazyGuidId { get; set; } }

This will cause the the id inferring to happen on CrazyGuid instead of Id.

Is the mismatch deliberate or is this just a typo: should the attribute refer to CrazyGuidId, or should the property name be CrazyGuid? There are other typos on the page so it's hard to tell ¯_(ツ)_/¯

I am having trouble getting the ID from items in ES using this method, using v1.4.2

---

EDIT: I came across this line on the same page:

Whenever an object is passed that needs to specify an id (i.e index, bulk operations) the object is inspected to see if it has an Id property and if so, that value will be used.

So the ID property only inferred from the type when using a .Id() search descriptor? If so, what's the recommended way of getting the _id property for documents from the Search api?

[rikkit]

Ok, resolved my issue

Given

``` c#
[ElasticType(IdProperty="Uid")]
class Poco {
public string Uid {get;set;}
public string Prop {get;set;}
}

var response = _client.Search(x => x
.Index(index)
.Type(type)
.Filter(f => f.Term(poco => poco.Prop)));

Instead of using `response.Documents`, where the `Uid` prop of each would be null, use:

``` c#
var results = response.Hits.Select(hit =>
            {
                var run = hit.Source;
                run.Id = hit.Id;
                return run;
            });

Easy enough. But I think the id property should _always_ be inferred and the value of _id transferred over, if [ElasticType(IdProperty="Uid")] is used.

[0063292]

+1 same issue, agree with " I think the id property should always be inferred and the value of _id transferred over, if [ElasticType(IdProperty="Uid")] is used."

There also seems to be an issue with id inference with the Bulk api.

[Mpdreamz]

This has always been deliberate, but I'm open to reevaluate this decision in 2.0.

The only time we infer Id is when writing out to elasticsearch but not when something comes back.

[irfansharif]

has there been any progress made on this?

``` C#
var response = client.Search(searchDescriptor);

var pocoListWithIds = response.Hits.Select(h =>
{
h.Source.Id = h.Id;
return h.Source;
}).ToList();
```

works fine but is not testable given the .Select is an extension method.

[Mpdreamz]

This is something that we will be implementing in NEST 2.0 but that won't be backported to 1.x.

[Mpdreamz]

Revisiting this I think i am OK with the current implementation:

IdProperty is about automatically inferring the id from objects when they need to be passed to urls or e.g bulk meta headers.

IdProperty refers to a property in your POCO that should ideally get stored in elasticsearch (and ideally as not analyzed for string id's). So when you are reading the data your id's are part of the POCO.

Something I missed in your original comment:

[ElasticType(IdProperty="Uid")]
class Poco {
public string Uid {get;set;}
public string Prop {get;set;}
}

var response = _client.Search<Poco>(x => x
                .Index(index)
                .Type(type)
                .Filter(f => f.Term(poco => poco.Prop)));

you mention:

Instead of using response.Documents, where the Uid prop of each would be null

Uid is not not null if an uid property exists wonder whats going on there in your mapping.

I'm +1 on keeping the current behaviour @russcam @gmarz do you feel differently?

[rikkit]

The uid property doesn't exist in my mapping, imo it shouldn't because it is already part of the document metadata. To reiterate my point, I expected this feature to be a "transparent" way to get the ID of a document _without_ having to have an id field stored in the document metadata.

As it stands, this feature is not useful for me, exactly because the read case differs from the write case.

If I put this document into ES using this mechanism:

``` c#
[ElasticType(IdProperty="Uid")]
class Poco {
[JsonIgnore] // alternatively to this attribute mapping could be explicit and dynamic mapping disabled
public string Uid {get;set;}
public string Prop {get;set;}
}

_nest.Index(x => x.Index(i).Type(t).Document(new Poco{ Uid = "banana", Prop = "apple" });

And then I retrieved it again:

``` c#
var poco2 = _nest.Get<Poco>(x => x.Index(i).Type(t).Id("banana");

I would not have the same object (poco1.Uid == banana, poco2.Uid == null).

What use cases does the current behaviour serve? It's not what I expected based on the documentation. And like @irfansharif says the workaround I posted is not easily testable.

[russcam]

Upon further discussion on this issue, we've decided to keep the current implementation for the time being. We feel it would be potentially confusing if the Id property on the Poco were populated with data that is not contained within the _source field in Elasticsearch; A Poco instance _is_ a representation of the source document.

I'll add documentation for NEST 2.x demonstrating how to retrieve the _id from the metadata using the .Hits property on the response and set a property on the Source as demonstrated above.

[gerosalesc]

Hello @russcam Can you address us to this part of the docs? found myself working around this problem by manually setting the ID of the source object before updating it but maybe there is a better way?

[russcam]

Thanks for the bump @gerosalesc, I haven't added a section to the documentation yet but will do so :smile: The way to do it is given by @rikkit above

``` c#
var response = client.Search(searchDescriptor);

var pocoListWithIds = response.Hits.Select(h =>
{
h.Source.Id = h.Id;
return h.Source;
}).ToList();
```

response.Documents is simply a shortcut to response.Hits.Select(h => h.Source).ToList(), so the example here is to map the _id from hits metadata to an Id property on each document source

[sove001]

hi, @russcam, i want to argue with you =)

If Poco is a representation of source document, why don't you remove functionality looks like:
[ElasticType(IdProperty="Uid")].
On th one hand Poco can be source + _id, but on the other hand we have to use this programming crutch.

I think that it is very common to make Id setter privateprotected.

[olee]

I agree with @sove001.
Currently there is an inconsistency in the usage of the id property.
There is no reason against adding this improvement.