On 27 May 2014, at 12:29, Hardy Ferentschik <ha...@hibernate.org> wrote:
> Thanks for the input. Comments inline.
>
> On 27 Jan 2014, at 12:01, Emmanuel Bernard <emman...@hibernate.org> wrote:
>
>>
>> On 06 May 2014, at 21:43, Hardy Ferentschik <ha...@hibernate.org> wrote:
>>
>>> Where are we at in this discussion?
>>>
>>> I think we basically have to main proposals.
>>>
>>> #1 Don’t include the embedded id per default. If @IndexEmbedded is used via
>>> the depth parameter there is no way to include the embedded id.
>>> In order to include the id you would need to use includePaths. depth and
>>> includePaths can be used in combination, so something like this
>>> is possible @IndexedEmbedded( depth = 1, includePaths = “foo.id” ). This
>>> will include all configured fields of the embedded entity, plus its id.
>>> #2 Don’t include the embedded id per default, but have an additional flag
>>> on @IndexedEmbedded to include the embedded id. The default would be false.
>>> To include the id would be something like @IndexedEmbedded( depth = 1,
>>> includeEmbeddedId = true ) or
>>> @IndexedEmbedded( includePaths = {“foo.a”, “foo.b”, “foo.c”} ,
>>> includeEmbeddedId = true ).
>>
>> I like #2 much better. It is too complex to ask of the user to understand
>> how the combination of depth and includePaths work based on fairly arbitrary
>> rules.
>> I thing the property should be named includeEmbeddedObjectsIds because
>> embeddedId has a precise meaning in ORM.
>
> Cool. Seems we have now several votes for this approach. I will start
> implementing it then.
> +1 also for ‘includeEmbeddedObjectsIds'
>
>> One question in that case, what does this one do
>>
>> @IndexedEmbedded( includePath = { “foo.id” }, includeEmbeddedId=false
>> //default )
>>
>> I think we should honour includePath according to the additive rule
>> mentioned earlier and the path of least surprise.
>
> Sounds reasonable.
>
>>> On top of this we seem to agree that it is a good idea to set the default
>>> depth value of @IndexedEmbedded to 0. This avoids the change in default
>>> values,
>>> when includePaths is used. As a side effect the default @IndexedEmbedded
>>> annotation becomes a no-op and should be treated as an invalid
>>> configuration.
>>> The choice here is between:
>>>
>>> #1 Log warning and move on
>>> #2 Throw an exception due to invalid configuration
>>>
>>> My choice would be #2 again.
>>
>> Is that really a good idea? As you guys said, @IndexedEmbedded becomes a
>> (silently or not) broken mapping :(
>> It also means that the simplest @IE usage requires to understand depth and /
>> or includePath.
>> I don’t like that at all. It makes things more complex than they should be
>> for the simple case.
>
> So what is your take on this then? Leave as is and keep the fact that the
> default depth value changes its default value depending
> on whether or not includePaths is used? That would be option
>
> #3 Keep status quo for value of depth parameter
>
> I raised the concern that the simple @IndexEmbedded is now not valid anymore
> as well before. I guess the question is what you give more importance,
> ability to use the annotation with its default values or have consistent
> default values which don’t change.
>
> I still think consistency is more important and #2 is the better approach.
> However, before going to #1, I would rather join you and keep the status quo
> with #3.
>
> Btw, enforcing a depth or includePath value might have the advantage of
> creating smaller (more targeted) indexes, since we less likely include
> fields which are need targeted by a query.
#3 then #2 for me. I really like #3 better though for the reason I explained.
We should bet at horse races together ;)
_______________________________________________
hibernate-dev mailing list
hibernate-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/hibernate-dev
