[ https://hibernate.onjira.com/browse/HSEARCH-1049?page=com.atlassian.jira.... ] Marc Schipperheyn commented on HSEARCH-1049: -------------------------------------------- Well, I wouldn't mind adding it explicitly as a field, but since this is in the current version not necessary, you should be able to use it in an includePath. Would adding a Field annotation solve this issue (regardless whether it may be considered a bug and addressed later)? -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://hibernate.onjira.com/browse/HSEARCH-1049?page=com.atlassian.jira.... ] Marc Schipperheyn commented on HSEARCH-1049: -------------------------------------------- Alright then. I'll be continuing testing as soon as maven spits out another nightly that addresses the issue. Cheers! -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://hibernate.onjira.com/browse/HSEARCH-1049?page=com.atlassian.jira.... ] Sanne Grinovero updated HSEARCH-1049: ------------------------------------- Fix Version/s: 4.1.0.Final -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://hibernate.onjira.com/browse/HSEARCH-1049?page=com.atlassian.jira.... ] Marc Schipperheyn commented on HSEARCH-1049: -------------------------------------------- Well, never delivered anything on Hibernate Search before. For Hibernate for that matter, but I'll have a go. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://hibernate.onjira.com/browse/HSEARCH-1049?page=com.atlassian.jira.... ] Marc Schipperheyn commented on HSEARCH-1049: -------------------------------------------- Ok, well, had a look. The reason that the id field member is not included in the "approved paths" seems to be that only fields are bound that have a Field annotation. As I'm not so familiar with the code, judging by the checkForField method, I would conclude that only Field annotated fields are eligible as includePaths. So, I tried adding a class bridge created field to an includePaths and sure enough, it bombed also. I'm not sure what the consequences are of binding a Field, but otherwise I would suggest that idFields should be "bound" also. Class level generated fields can probably be tied in to whatever mechanism makes sure we can search on them now. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://hibernate.onjira.com/browse/HSEARCH-1049?page=com.atlassian.jira.... ] Davide D'Alto commented on HSEARCH-1049: ---------------------------------------- Hi Marc, I think I have a fix for this issue: https://github.com/DavideD/hibernate-search/commits/HSEARCH-1049 Please, could you try it and let me know if it works? Thanks, Davide -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://hibernate.onjira.com/browse/HSEARCH-1049?page=com.atlassian.jira.... ] Marc Schipperheyn commented on HSEARCH-1049: -------------------------------------------- Ad 4 org.hibernate.search.SearchException: Unable to find field subitem.item.id in nl.project.model.Item at org.hibernate.search.engine.spi.DocumentBuilderIndexedEntity.objectToString(DocumentBuilderIndexedEntity.java:746) at org.hibernate.search.query.dsl.impl.ConnectedMultiFieldsTermQueryBuilder.buildSearchTerm(ConnectedMultiFieldsTermQueryBuilder.java:141) at org.hibernate.search.query.dsl.impl.ConnectedMultiFieldsTermQueryBuilder.createQuery(ConnectedMultiFieldsTermQueryBuilder.java:95) at org.hibernate.search.query.dsl.impl.ConnectedMultiFieldsTermQueryBuilder.createQuery(ConnectedMultiFieldsTermQueryBuilder.java:76) -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://hibernate.onjira.com/browse/HSEARCH-1049?page=com.atlassian.jira.... ] Marc Schipperheyn commented on HSEARCH-1049: -------------------------------------------- Ok, figured out a fix but I need some help in evaluating the consequences. This also fixes querybuilder exception AbstractDocumentBuilder private PathsContext updatePaths(String localPrefix, PathsContext pathsContext, IndexedEmbedded embeddedAnn) { //This was preventing embedded paths from being added to the pathsContext // if ( pathsContext != null ) // return pathsContext; PathsContext newPathsContext = new PathsContext( embeddedAnn ); for ( String path : embeddedAnn.includePaths() ) { newPathsContext.addPath( localPrefix + path ); } return newPathsContext; } I'm just not sure it removing this will have unwanted side effects. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://hibernate.onjira.com/browse/HSEARCH-1049?page=com.atlassian.jira.... ] Marc Schipperheyn commented on HSEARCH-1049: -------------------------------------------- Well, I'm just running your github version against my production code. So, no, I didn't try building it. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://hibernate.onjira.com/browse/HSEARCH-1049?page=com.atlassian.jira.... ] Davide D'Alto commented on HSEARCH-1049: ---------------------------------------- Looks great, I'm going to take a better look at it in the week end but if it passes all the tests is a very good sign. In the meanwhile... It would be awesome if you could create a fork of the project and apply the fix with some tests, this way someone else won't take credit for you effort. Thanks for the help. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://hibernate.onjira.com/browse/HSEARCH-1049?page=com.atlassian.jira.... ] Sanne Grinovero commented on HSEARCH-1049: ------------------------------------------ Hi Marc, I've found branch HSEARCH-1049 on https://github.com/mschipperheyn/hibernate-search . Is that the correct place? I couldn't find the tests Davide suggested. Could you please add a test? That's an important aspect of a patch, as it makes sure the fix works now and above all that it will work as you expect in the future. All functionality needs to be covered by tests, otherwise when someone will introduce another feature or do some cleanup (which I'm doing on this area!) a single line of code like that might be overzealous removed. There should be a failing test if I remove your last commit. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://hibernate.onjira.com/browse/HSEARCH-1049?page=com.atlassian.jira.... ] Davide D'Alto commented on HSEARCH-1049: ---------------------------------------- {quote} Sorry, yeah I'm new to this. I ran the engine tests since that's what I worked on ...{quote} Don't worry Marc. Another reson to add tests is this way is easier for us to understand your use case. Anyway I think I've understand what's going on: 1) "subitem.item.id" doesn't exist because the includePaths option create paths from the class where is used. Other includePaths in embedded entities are not considered. This is necessary to avoid infinite loop with some mapping. So {code} @Indexed public class Item { @ManyToMany @IndexedEmbedded(prefix="subitem.") public List<SubItem> getSubItems() { ... } } {code} {code} public class SubItem { @IndexedEmbedded(prefix="item.",includePaths={"id"}) public List<Item> relatedItems() { ... } {code} -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://hibernate.onjira.com/browse/HSEARCH-1049?page=com.atlassian.jira.... ] Davide D'Alto commented on HSEARCH-1049: ---------------------------------------- Sorry I pressed enter too soon... There is a problems in this case: Because you have specified includePaths in Subitem the depth is set to 0 and therefore the relatedItems are not indexed (they are indexed when creating the index for SubItem under item.id). subitem.item.id is going to be created if you specified it in Item with: {code} @IndexedEmbedded(prefix="subitem.", includePaths={"id", "item.id"}) {code} Another problem is that HibernateSearch is actually loading all the JPA classes when indexing and not only the one annotated with @Indexed (is this correct Sanne?), because of this something goes wrong in the validation of the paths. I think we should disable it in the class: DocumentBuilderContainedEntity.java I'm not sure if I have been clear but it's a quite complicated part. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://hibernate.onjira.com/browse/HSEARCH-1049?page=com.atlassian.jira.... ] Davide D'Alto commented on HSEARCH-1049: ---------------------------------------- The depth = 0 is only a problem for the index created from Item. In the SubItem index the fields "id" and "item.id" should be present. Are you saying that this is not the case? Could you let as know how the SubItem class is annotated? -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://hibernate.onjira.com/browse/HSEARCH-1049?page=com.atlassian.jira.... ] Davide D'Alto commented on HSEARCH-1049: ---------------------------------------- I'm not sure but could it be related to the fact the both are missing the @ContainedIn annotation? -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://hibernate.onjira.com/browse/HSEARCH-1049?page=com.atlassian.jira.... ] Davide D'Alto commented on HSEARCH-1049: ---------------------------------------- Great! So, the patch seems to fix the problem of the indexing of ids but we discovered another bug related to the depth. Would you like to propose a solution for this second one? I think it's not hard. It should be fix by checking the paths only if the root @IndexEmbedded define the attribute includePaths. So you just have to change the updatePaths(...) method. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://hibernate.onjira.com/browse/HSEARCH-1049?page=com.atlassian.jira.... ] Davide D'Alto commented on HSEARCH-1049: ---------------------------------------- If Mark confirm that with the patch I've submitted this issue is solved I can do the pull request this evening (London time). -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://hibernate.onjira.com/browse/HSEARCH-1049?page=com.atlassian.jira.... ] Marc Schipperheyn commented on HSEARCH-1049: -------------------------------------------- @David Perhaps we should resolve this and I will create a second Jira item for the other issue? Or should we keep this open here? Just to summarize: the behaviour is that if SubItem is indexed from the root, the SubItem index itself will only contain those fields that were specified in the root of Item. However, SubItem itself as a *root item* may have a different path configuration and require different fields. I'm not sure if this should be considered a bug or not since the whole point is to avoid unnecessary index paths. So, probably not. In any case, the result is that the SubItem index itself will only be guaranteed to be in a correct state if you save the relevant SubItems directly or MassIndex it. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://hibernate.onjira.com/browse/HSEARCH-1049?page=com.atlassian.jira.... ] Marc Schipperheyn commented on HSEARCH-1049: -------------------------------------------- Yeah, I was confused as well and trying to spell out what I thought you were saying. Anyway, I condensed my ideas about the "issue", if there is any, in my latest comment. As far as I can see, this is a natural downside of the includePaths option. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://hibernate.onjira.com/browse/HSEARCH-1049?page=com.atlassian.jira.... ] Marc Schipperheyn commented on HSEARCH-1049: -------------------------------------------- Hey, where did all the comments go? -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://hibernate.onjira.com/browse/HSEARCH-1049?page=com.atlassian.jira.... ] Sanne Grinovero updated HSEARCH-1049: ------------------------------------- Fix Version/s: (was: 4.1.0.CR2) 4.1.0.Final -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://hibernate.onjira.com/browse/HSEARCH-1049?page=com.atlassian.jira.... ] Sanne Grinovero updated HSEARCH-1049: ------------------------------------- Priority: Major (was: Blocker) Assignee: Davide D'Alto -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://hibernate.onjira.com/browse/HSEARCH-1049?page=com.atlassian.jira.... ] Sanne Grinovero commented on HSEARCH-1049: ------------------------------------------ Marc and Davide, the pull request was mentioning "other problems", could you make sure they are tracked on JIRA (even if you're not rushing to fix them)? Marc, on your last documentation suggestion.. I'm surprised if that doesn't work, that should be triggered automatically by @ContainedIn ? Maybe I didn't understand it. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://hibernate.onjira.com/browse/HSEARCH-1049?page=com.atlassian.jira.... ] Sanne Grinovero commented on HSEARCH-1049: ------------------------------------------ Yes I agree with your expectations, but since "includePaths" is relatively young, I'm not sure how many tests cover for this, we should improve on that side and you're welcome to add some. That's the only way I know to answer you "that works". If we're speaking about the API intentions: includePaths is an extension to @IndexedEmbedded, so yes it definitely defines which fields are included and which are not, but as it was with the "depth" strategy or any older @IndexedEmbedded, to have its fields updated by a change in one of the entities affecting the indexed path, you would need @ContainedIn to trigger the update, walking the path backwards. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://hibernate.onjira.com/browse/HSEARCH-1049?page=com.atlassian.jira.... ] Marc Schipperheyn commented on HSEARCH-1049: -------------------------------------------- Just wanted to touch base if an issue I ran into should become a new JIRA item. I ran into an issue and reviewed some scenario's of using id properties. Some of the scenario's I wasn't sure if what I got was to be expected and one scenario might be a real problem Scenario's Item: Specify an IndexedEmbedded with includePaths id property on a SubItem, no @Indexed on the SubItem - OK Item: Specify an IndexedEmbedded with includePaths id property on a SubSubItem of SubItem, no @Indexed on the SubItem, no @IndexedEmbedded on SubItem:SubSubItem - FAIL Item: Specify an IndexedEmbedded with includePaths id property on a SubSubItem of SubItem, no @Indexed on the SubItem, @IndexedEmbedded with depth=x on SubItem:SubSubItem - OK Item: Specify an IndexedEmbedded with includePaths id property on a SubSubItem of SubItem, no @Indexed on the SubItem, @IndexedEmbedded with includePaths with an id property on SubItem:SubSubItem - FAIL Item: Specify an IndexedEmbedded with includePaths id property on a SubSubItem of SubItem, @Indexed on the SubItem, @IndexedEmbedded with includePaths with an id property on SubItem:SubSubItem - FAIL This last one is a problem because it basically means that if an Entity is part of a includePaths and is also a an Indexed Entity itself, it cannot includePaths with id properties itself Item: Specify an IndexedEmbedded with includePaths id property on a SubSubItem of SubItem, @Indexed on the SubItem, @IndexedEmbedded with includePaths with a normal property on SubItem:SubSubItem - OK Item: Specify an IndexedEmbedded with includePaths id property on a SubSubItem of SubItem, no @Indexed on the SubItem, @IndexedEmbedded with includePaths with a normal property on SubItem:SubSubItem - OK -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira