[hibernate-dev] [Search] Index embedded and id property of embedded entity

[Sanne Grinovero]

On 29 April 2014 12:45, Hardy Ferentschik <hardy at hibernate.org> wrote:
>
>>>> Question: would we still interpret the literal "id" as a keyword
>>>> pointing to whatever getter is the primary key of the embedded object?
>>>
>>> Why still? We don’t do this at the moment afaict. Our examples and tests just use ‘id’
>>> all the time.
>>
>> Right I got confused. I thought for a moment that we would encode the
>> primary identifier as a specific keyword, but there's no reason.
>> Forget what I said :)
>
> :-)
>
>>>> I'm inclined to seek for a property which has the name as listed in
>>>> _includePaths_, not giving the "id" literal a special treatment in
>>>> this case.
>>>
>>> I am not following you here. What do you mean? Also you seem to just target the includePath case.
>>
>> Yes I mean we could never include it by default, and allow the
>> "includePath" to be the (only) way to include things.
>
> But this forces you to list all fields to include explicitly in case you want the id added, but otherwise are happy
> to just use the default @IndexedEmbedded.
>
>> Remember _includePath_ is additive to fields otherwise included via
>> @IndexedEmbedded _depth_, so it fits nicely for this role:
>
> Is that really true? AFAIK the includePath attribute changes the behaviour of the depth attribute.
> Depth is ‘0’ per default if ‘includePath’ is used. So you would have to include the id via includePath
> and also set the depth explicitly to something > 0.

No it's additive. depth=n will add "all" @Field until depth=n, and it
*also* will add what is explicitly listed in includePath.
So asserting that "all" doesn't include fields which are missing
Hibernate Search indexing annotations is a quite natural implication.

The problem if any is that the default depth value is different
depending if you use includePath or not, but also the alternative
Integer.MAX_VALUE doesn't make much sense in practice as noone will
define queries on an unbounded path.

I'd actually like to propose to change the depth default to zero, and
since includePath also defaults to an empty list, we'd be logging a
warning as the @IndexedEmbedded annotation would have no effect.


> I also find it not intuitive that this means:
> "index all embedded fields up to the given depth, plus the specified paths”.
> I never liked how we baked depth and includePath into the same annotation. A dedicated annotation would
> have been more appropriate.

We've been there and couldn't agree on a better proposal. Feel free to
reopen the case on a new thread, if you have a nice name in mind. But
ultimately remember the goal is to allow queries on a well-known list
of field names, and it would be great to validate for these queries,
so to have a clear definitions of which indexing and analysis options
are applied to each Lucene field, and how to apply a bi-directional
projection.. so I'm still skeptical on leaving too much freedoom, or
have multiple ways to achieve the same thing.

>> for non-indedex embedded elements it's of course nonsesense (as usual)
>> to specify an attribute which is lacking a @Field or similar option,
>> while for indexed types we implicitly apply one on the primary
>> identifier, this just needs to be selectable via _includePath_.
>
> I am just not sure, whether we easily can tell when processing the embedded entity whether it is indexed or not.
> Obviously that is an implementation detail, but we might have to jump through some hoops here (might be wrong).

right. I guess we'll have to try.

>
>>> Another thought would be to make this a global configuration option. Something like
>>> hibernate.search.embeddedindexing.include_id_property. Per default the flag would be false, but
>>> could be set to true. Obviously this is a quite coarse solution.
>>
>> I wouldn't do that now, but we can certainly leave this option open
>> for future evolution.
>
> Ok.
>
> —Hardy