Hi, I am not sure I understand the suggested path parameter. Taking the example from the forum: class A{ @IndexEmbedded( paths={"d.one", "d.two"} ) private C see; } class B{ @IndexEmbedded( paths={"foo", "d.two"} ) private C see; } class C{ @IndexEmbedded private Collection<D> d; @Field private int foo; } class D{ @Field int one; @Field int two; } what does it do? What fields get indexed for each entity? --Hardy On Fri, 19 Aug 2011 11:07:07 +0200, Sanne Grinovero <sanne(a)hibernate.org&gt; wrote:

On Mon, 22 Aug 2011 12:58:00 +0200, Sanne Grinovero <sanne(a)hibernate.org&gt; wrote: Now I get it. We are only talking about which fields get included. I somehow over-interpreted and thought you can also change how the fields are indexed (different field configuration from within @IndexedEmbedded). If we are just talking about field inclusion I think this approach is reasonable. I assume 'paths' would be optional and if not specified the current approach applies, right? That the other thing. Wouldn't you expect the path being the field names? I bet you we go for this syntax we get heaps of questions about the search not working simply because they search for the wrong field name. --Hardy

2011/8/22 Emmanuel Bernard <emmanuel(a)hibernate.org&gt;: Right. I think we should support entities having class level bridges as @IndexedEmbedded, by prefixing whatever they want to add to the index with the current prefix/navigation path, in a similar fashion of what we do with MaskedProperty. This implies removing direct access to the Document from the FieldBridge API org.hibernate.search.bridge.FieldBridge.set(String, Object, Document, LuceneOptions) and have APIs write to a surrogate Document. This is a radical change, and I wouldn't suggest it if it where for this feature only, but we have found more reasons to introduce the surrogate Document in the past: - Dirty checking optimizations would require a FieldBridge API change to know which entity fields are being read (which might be dirty, or unchanged) - To enable at some point - not necessarily for 4.0.0, but possibly without breaking APIs in 4.x - the capability to perform Analysis before sending the Document to the backends This is one of the points we have on http://community.jboss.org/wiki/PlansForHibernateSearch4 but of course it's not cast in stone, especially since I previously thought we would need it for the Serialization API but Emmanuel proved otherwise. Sanne

On 22 août 2011, at 14:07, Hardy Ferentschik wrote: You can't do that. At least you need a (conceptual) Set<Fieldable>.

Hi, I was the poster on the forum that triggered this conversation. Sanne suggested I jump onto the dev mailing list for this and future suggestions, so here I am. I'm a developer in the San Francisco bay area. I've been using hibernate search for about a year on a project I've been working on. Primarily to eliminate lots of expensive joins, and leveraging wild carding, but not many other full text features(phonetic searching, stemming, etc)... yet. In any case, back to the topic at hand: I think specifying 'see.d.one' is redundant, since the annotation is on the property 'see'. I like the subPaths idea, it makes the lack of needing to specify 'see' more intuitive. It just may be my lack of use of class bridges, but I would think it may be reasonable they be ignored 'within' subPaths, but OK if a path terminates with a type that has a ClassBridge specified. Even if a property along the path has a class level bridge, I'd expect it to not be invoked and that the specific path is still followed with the default indexing behavior. Or at the very least that would be a straight forward starting point for the behavior. Another related suggestion, is that in many cases people may want fine grained control from an exclusion point of view. I'm usually trying to target very specific fields to search on, but there are cases where I've needed a lot of things to be indexed and the default behavior has been quite nice from a development time savings perspective. Even though its just a one time dev cost, it might be nice if folks could specify an 'excludeSubPaths' option as well. Where the default behavior might work fine for them for 99% of what is being indexed, but a particular path is causing a problem and doesn't need to be indexed for a particular use case. Still though, if I had to choose only one, specifying 'subPaths' would be a very nice explicit tool for only indexing what I actually need per use index use case. Zach

On Aug 24, 2011, at 8:26 AM, Sanne Grinovero wrote: Yes, if 'excludeSubPaths' is provided, then 'subPaths' should be renamed to 'includeSubPaths', for cleanliness/symmetry sake. I think wasn't exact enough. I would expect 'includeSubPaths' to be incompatible with both 'depth' and 'excludeSubPaths'. However, I would expect 'depth' and 'excludeSubPaths' to be compatible. Which basically says to index using the default approach, and only stop at max depth, but exclude indexing of the paths specified. given: class C{ @IndexEmbedded private Collection<D> d; @Field private int foo; } Illegal configuration: can't specify depth and includeSubPaths simultaneously: class A{ @IndexEmbedded( includeSubPaths={"d.one", "d.two"}, depth=5 ) private C see; } Illegal configuration: specifying includeSubPaths and excludeSubPaths is nonsense, since absence of specifying in includeSubPaths means the path won't be indexed anyway: class A{ @IndexEmbedded( includeSubPaths={"d.one", "d.two"}, excludeSubPaths {"d.three"} ) private C see; } Valid configuration: Excludes indexing of d. Maybe D leads to cycles, or expensive nested joins, and it isn't used when searching index A, so we want to exclude it. class A{ @IndexEmbedded( depth=5, excludeSubPaths {"d"} ) private C see; } Also what validly constitutes a path is different for excludeSubPaths. Anywhere in a 'path' can be a termination point where the user can express that they don't want indexing to go down that path any further; and that could potentially go down to a leaf. While 'includeSubPaths' must be composed of leaf nodes. No, that wasn't what I meant. I'd expect the annotation processing to basically look like: IndexEmbedded embeddedConfig = (IndexEmbedded) node.getAnnotation(IndexEmbedded.class); if(embeddedConfig.includeSubPaths() != null && embeddedConfig.depth() != null || embeddedConfig.excludeSubPaths() != null){ throw new IllegalArgumentException("Invalid configuration: Cannot specify includeSubPaths and depth(nor excludeSubPaths), simultaneously"); } Hopefully it would be understood that if only includeSubPaths is provided, then the default depth is irrelevant and is explicitly expressed per path. I think the complex option you thought I was implying was a mixed bag approach. Which I'm not advocating for. My only purpose for suggesting the 'exclude' option is that if I have 100 properties I want to index for a particular entity, then listing 100 properties explicitly in 'includeSubPaths' could be laborious(and some might think messy). Those 100 properties could be directly on the entity, or they could be through associated entities. However, because of my desire to have those 100 properties, because of 'depth' I might end up with 1000 values indexed(mostly waste and potentially costly). In that case maybe those 900 other values come from a particular unused path, or a path I can prune a bit through via 'excludeSubPaths'. Overall I think the options of: default approach, default + excludesSubPaths, or includeSubPaths(but no default depth or excludes), gives users 3 good options for how they want to go about indexing, and they can choose the least painful approach for their particular use case. Most cases are going to be simple and for a particular entity only a very limited subset of properties are needed for search, and I'd probably go with 'includeSubPaths' most of the time in our particular object model. Hope that clarifies things? Zach

On 25 août 2011, at 11:28, Hardy Ferentschik wrote: +1

On Thu, 25 Aug 2011 17:47:30 +0200, Zach Kurey <pushedbytime(a)gmail.com&gt; wrote: I think there was never a question of what exactly is included or excluded. The question is whether the property name of the property the annotation is defined on should be part of the path. The advantage of adding it is that it reflects the actual field name being indexed, but I can also see the motivation not to add it. Yes, one of these alternatives. Maybe the "one new 'exclude' attribute in IndexEmbedded and one new annotation for IndexSubPaths" It still conflicts with the actual default value of depth and that there is no explicit way to say that it is ignored. I don't think "it seems clear" is a good enough reason. Exactly. I get that and obviously you want to fine tune this as much as possible. Just wanted to make sure you do it for all the right reasons :-) --Hardy

I think the original proposal was great, but also that we're now pushing it to accomplish too much. This feature as the potential to be *brilliant* but it must be striking simple to both understand and use. So what I like of the original concept, is that people using it will focus on what they need starting from the concept of query, instead of from the mapping: the mapping is a way to enable the queries, and the queries are your goal. When I'm coding, I know which Query I'll need to implement my feature, and usually I will adjust index mapping consequentially; currently when using @IndexedEmbedded on complex object graphs I might need to draw a sketch to visualize the graph and define a proper depth (and as you say I often ended up with custom ClassBridges to get better control on objects loading). So IMO the goal should be that starting from a well defined concept of the FullTextQuery I'm going to need, I'm able to define the mapping in a straight-forward form (and optimal, performance wise) to satisfy these queries. I think we can achieve this, but to be explicit with the mapping from the queries I won't need the exclusion feature. I understand that it might save some lines of code, but it will make my design process more complex as I'll have to think if I'll get my desired mapping easier with depth N and remove some paths, or depth N-1 and add some paths... I'd prefer to have no options and keep it simple: - if the developer is not sure yet about which fields he'll need next day, he should set a reasonably high depth and don't bother about the paths yet. - when requirements have settled down, and it's time for performance tuning, he'll be able to lower the defined depth values replacing them with explicit paths. This step should be easy: set all depths to zero, add the paths you think you're using, run tests, repeat until all queries are satisfied. Proposal: we add an array of @IndexedPaths to @IndexedEmbedded; depth remains the same, so for example @IndexedEmbedded(depth = 1, paths= {(a)IndexedSubPath(&quot;livingAddress.countryCode&quot;), @IndexedSubPath("bornPlace.countryCode")}) private Person ownedBy; Or just @IndexedEmbedded(depth = 1, paths={"livingAddress.countryCode", "bornPlace.countryCode"}) private Person ownedBy; I like the second more for it's shorter to write, but the first one can be extended later. Let's go by steps and ignore the "paths" name for now. While I'm obviously not so excited about the exclusions idea, maybe I'm overseeing a very useful case; is there a compelling reason to want that, enough to justify the added complexity? I'm not so much concerned about our code complexity (which might be an issue too) but more about the thinking process one must apply to use it. Regards, Sanne 2011/8/25 Zach Kurey <pushedbytime(a)gmail.com&gt;:

@IndexPath allows for further extension, but I could live with simple strings as well. +1 --hardy

Hi Zach, I'm not 100% convinced but please go ahead and create a JIRA. There are definitely good improvements to take out of this thread, we can flesh out the details on JIRA or after a proof of concept patch. My doubts is that I don't like adding an alternative to @IndexedEmbedded which has the same functionality; instead of creating a new one we could add an array of @IndexPaths to the @IndexedEmbedded (to be interpreted as additional paths), but I'd prefer the string in this case just because I would expect to use this often, and typing an array of annotations as a parameter of another annotation is just ugly :) But a part from my taste, I'm not against this, the feature looks very good and useful, so let's open the issue and see if we get better ideas while implementing it. Regards, Sanne

2011/8/30 Zach Kurey <pushedbytime(a)gmail.com&gt;: [...] Yes I understood that was your suggestions; but I think that using @IndexEmbedded(depth=0, {IndexPath1, IndexPath2}) would get you exactly what you wanted? Just that I can still use for example depth=1 combined with some paths to mean "all of depth 1, plus these extra paths we hand-picked". This flexibility seems handy to me, and makes the two features compatible (not conflicting in any meaning), to the point that IMHO they don't need new annotations. Sanne

On Aug 30, 2011, at 11:51 AM, Zach Kurey wrote: Created https://hibernate.onjira.com/browse/HSEARCH-886 Thanks, ZK