[ https://issues.apache.org/jira/browse/SOLR-7888?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14705925#comment-14705925 ]
Arcadius Ahouansou edited comment on SOLR-7888 at 8/20/15 10:53 PM: -------------------------------------------------------------------- Hello [~janhoy] Thank you very much for your comments. Have uploaded new version of the patch. {quote}Perhaps this property should be moved to some other Lucene class as a common global name for context field for all analyzers that supports context filtering,...{quote} I agree and I moved {{CONTEXTS_FIELD_NAME}} into Lucene's {{Lookup.java}}, meaning that it is now available to all Lookup implemetations. {quote}Regarding a request including suggesters that do not support filtering, I think it depends on its data whether the correct thing is to return an unfiltered response (open data) or to return nothing (sensitive data). Of course, the application has the power to pass suggest.dictionary accordingly if it knows that filtering is done. Alternatively, some suggest.returnUnFilteredSuggestionsIfFilteringIsNotSupported param to control it, I don't know...{quote} Not quite sure about this: Let's take the current solr 5.2.1: passing {{suggest.q=term&suggest.contestFilterQuery=ctx1}} will return all suggestions matching the {{term}} ignoring {{ctx1}} as context filtering is not yet implemented. I believe that keeping that behaviour for Lucene Suggesters that have not yet implemented context makes more sense to me. In case a user need context filtering to happen on a Lucene suggester not yet supporting filtering, they just need to implement it. Ideally and eventually, we will have context support for all Lucene suggesters, so I am not quite sure whether {{suggest.returnUnFilteredSuggestionsIfFilteringIsNotSupported}} is the way we should go. {quote} I think that if CONTEXT_ANALYZER_FIELD_TYPE is explicitly given and wrong, we should fail-fast and throw exception instead of falling back to DocumentDictionaryFactory.CONTEXT_FIELD{quote} I had thought a bit more about this. I believe that we do not really need the {{CONTEXT_ANALYZER_FIELD_TYPE}} config. One just needs to configure the context field in {{schema.xml}} with the needed query and index analyzers and all should work. In case one needs different context analyzers for different suggesters, we just need to configure different context fields in {{schema.xml}}. This has several advantages: - Simpler/less configuration. - Cleaner/more readable/less code to maintain. In case I am missing any use-case, please let me know {quote} Will let others chime in on the param names too. Which one do you like the best? suggest.contextFilterQuery suggest.contextQ suggest.fq suggest.context.fq {quote} The param name is this latest patch is still {{suggest.contextFilterQuery}} as we have not agreed yet on the right name to adopt. Maybe [~rcmuir ] or [~shalinmangar] or [~varunthacker] could help here was (Author: arcadius): Hello [~janhoy] Thank you very much for your comments. Have uploaded new version of the patch. {quote}Perhaps this property should be moved to some other Lucene class as a common global name for context field for all analyzers that supports context filtering,...{quote} I agree and I moved {{CONTEXTS_FIELD_NAME}} into Lucene's {{Lookup.java}}, meaning that it is now available to all Lookup implemetations. {quote}Regarding a request including suggesters that do not support filtering, I think it depends on its data whether the correct thing is to return an unfiltered response (open data) or to return nothing (sensitive data). Of course, the application has the power to pass suggest.dictionary accordingly if it knows that filtering is done. Alternatively, some suggest.returnUnFilteredSuggestionsIfFilteringIsNotSupported param to control it, I don't know...{quote} Not quite convinced about this: Let's take the current solr 5.2.1: passing {{suggest.q=term&suggest.contestFilterQuery=ctx1}} will just return all suggestions matching the {{term}} ignoring {{ctx1}} as context filtering is not yet implemented. I believe that keeping that behaviour for Lucene Suggesters that have not yet implemented context makes more sense to me. In case a user need context filtering to happen on a Lucene suggester not yet supporting filtering, they just need to implement it. Ideally and eventually, we will have context support for all Lucene suggesters, so I am not quite sure whether {{suggest.returnUnFilteredSuggestionsIfFilteringIsNotSupported}} is the way we should go. {quote} I think that if CONTEXT_ANALYZER_FIELD_TYPE is explicitly given and wrong, we should fail-fast and throw exception instead of falling back to DocumentDictionaryFactory.CONTEXT_FIELD{quote} I had thought a bit more about this. I believe that we do not really need the {{CONTEXT_ANALYZER_FIELD_TYPE}} config. One just needs to configure the context field in {{schema.xml}} with the needed query and index analyzers and all should work. In case one needs different context analyzers for different suggesters, we just need to configure different context fields in {{schema.xml}}. This has several advantages: - Simpler/less configuration. - Cleaner/more readable/less code to maintain. In case I am missing any use-case, please let me know {quote} Will let others chime in on the param names too. Which one do you like the best? suggest.contextFilterQuery suggest.contextQ suggest.fq suggest.context.fq {quote} The param name is this latest patch is still {{suggest.contextFilterQuery}} as we have not agreed yet on the right name to adopt. Maybe [~rcmuir ] or [~shalinmangar] or [~varunthacker] could help here > Make Lucene's AnalyzingInfixSuggester.lookup() method that takes a > BooleanQuery filter parameter available in Solr > ------------------------------------------------------------------------------------------------------------------ > > Key: SOLR-7888 > URL: https://issues.apache.org/jira/browse/SOLR-7888 > Project: Solr > Issue Type: New Feature > Components: Suggester > Affects Versions: 5.2.1 > Reporter: Arcadius Ahouansou > Assignee: Jan Høydahl > Fix For: 5.4 > > Attachments: SOLR-7888.patch, SOLR-7888.patch, SOLR-7888.patch > > > LUCENE-6464 has introduced a very flexible lookup method that takes as > parameter a BooleanQuery that is used for filtering results. > This ticket is to expose that method to Solr. > This would allow user to do: > {code} > /suggest?suggest=true&suggest.build=true&suggest.q=term&suggest.contextFilterQuery=contexts:tennis > /suggest?suggest=true&suggest.build=true&suggest.q=term&suggest.contextFilterQuery=contexts:golf > AND contexts:football > {code} > etc > Given that the context filtering in currently only implemented by the > {code}AnalyzingInfixSuggester{code} and by the > {code}BlendedInfixSuggester{code}, this initial implementation will support > only these 2 lookup implementations. -- This message was sent by Atlassian JIRA (v6.3.4#6332) --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org