@IndexedEmbedded.includePaths should not allow to pick paths that were excluded from the embedded entity

Description

*WARNING*: There are use cases for this, so we might consider not removing the feature after all, but simply ensuring it works correctly. See HSEARCH-1112. => It is already done in 6 and throroughly tested. Let’s push some comments to these tests to link to this ticket, close this ticket, and reconsider when we have a look at HSEARCH-1112.

Follow-up on HSEARCH-2547.

We don't prevent upper-level restrictions to be broader than nested restrictions. For instance:

This will include b.a.bar in the resulting document for C, even though a.bar is not part of the document for B.

Environment

None

Activity

Show:
Yoann Rodière
January 18, 2017, 9:18 AM

Comments by on the PR for HSEARCH-2547:

That's a very interesting point. From one side, I agree with you that this shouldn't happen. On the other, I can see how someone needing to "fine tune" things might want to do that, so if we "fix" it by not doing it, we might need to introduce a specific feature for such a mapping.

You guessed right I don't think we should merge the second change at this stage; could you split that in its own issue, so then we can decide if it's material for 6?

I guess the culprit is to clarify if @IndexedEmbedded(includePaths) is a narrowing filter, or indicative.

When creating it I strictly had the narrowing idea in mind, but I can see how the include Paths terminology might have misled some people. For what it's worth, I realize this feature has been around for some time but I still feel like it's "bolted on" as we never revisited it from the first POC.. it's been a long time that I want to rethink the whole DocumentBuilder process from scratch.

Yoann Rodière
April 11, 2018, 8:14 AM

Original PR on Search 5 (might be superseded in 6): https://github.com/hibernate/hibernate-search/pull/1288/files

Yoann Rodière
April 25, 2018, 9:33 AM

I think this is solved in the Search 6 proof of concept, but we need to add a test in IndexSchemaNestingContextImplTest.

Yoann Rodière
September 18, 2018, 12:43 PM

*WARNING*: There are use cases for this, so we might consider not removing the feature after all, but simply ensuring it works correctly. See HSEARCH-1112.

It is already done in 6 and throroughly tested. Let’s push some comments to these tests to link to this ticket, close this ticket, and reconsider when we have a look at HSEARCH-1112.

Assignee

Yoann Rodière

Reporter

Yoann Rodière

Labels

None

Suitable for new contributors

None

Feedback Requested

None

Components

Fix versions

Affects versions

Priority

Major
Configure