[ 
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

Reply via email to