[GitHub] lucene-solr pull request #375: LUCENE-8287: Ensure that empty regex completi...

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] lucene-solr pull request #375: LUCENE-8287: Ensure that empty regex completi...

Bruno86
GitHub user jtibshirani opened a pull request:

    https://github.com/apache/lucene-solr/pull/375

    LUCENE-8287: Ensure that empty regex completion queries always returns no results.

    This ensures parity with the standard `RegexpQuery`, which returns no results for an
    empty regex. It also fixes the exception described in the ticket when an empty regex
    is provided to a context query, as we no longer try to match on an empty prefix.
   
    In addition to the fix, I added tests that check the behavior of an empty `RegexpQuery` and `PrefixCompletionQuery`.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/jtibshirani/lucene-solr bugfix/regex-completion-query

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/lucene-solr/pull/375.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #375
   
----
commit 88cbbb7b4e3a5eddc0482120f271c327f9362746
Author: Julie Tibshirani <julie.tibshirani@...>
Date:   2018-05-09T06:10:09Z

    LUCENE-8287: Ensure that empty regex completion queries always return no results.
   
    This ensures parity with the standard RegexpQuery, which returns no results for an
    empty regex. It also fixes the exception described in the ticket when an empty regex
    is provided to a context query, as we no longer try to match on an empty prefix.

----


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] lucene-solr issue #375: LUCENE-8287: Ensure that empty regex completion quer...

Bruno86
Github user jtibshirani commented on the issue:

    https://github.com/apache/lucene-solr/pull/375
 
    @jimczi I tried a few different approaches (including updating `RegexCompletionQuery#rewrite`), and this one turned out to be cleanest + most consistent. I'm happy to give more details about the alternatives if it'd be useful while reviewing.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] lucene-solr issue #375: LUCENE-8287: Ensure that empty regex completion quer...

Bruno86
In reply to this post by Bruno86
Github user jtibshirani commented on the issue:

    https://github.com/apache/lucene-solr/pull/375
 
    I'm sorry for the confusion -- I didn't mean complete parity with `RegexpQuery`, as they are different queries with different behaviors, but rather consistency how they treat an empty regex. After thinking more I now see your point, it makes a lot more sense to describe it as consistency with `PrefixCompletionQuery`!
   
    I've made the following changes:
    - Replaced references to 'parity with `RegexpQuery`' to 'consistency with `PrefixCompletionQuery`, as this is more accurate.
    - Added an optimization to `ContextQuery#createWeight` that returns an empty automaton if the inner weight is empty. Note that this optimization only helps with `RegexCompletionQuery`. When `PrefixCompletionQuery` contains an empty term, its automaton is actually `*` -- maybe we could look into adding an optimization there in a follow-up.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] lucene-solr issue #375: LUCENE-8287: Ensure that empty regex completion quer...

Bruno86
In reply to this post by Bruno86
Github user rmuir commented on the issue:

    https://github.com/apache/lucene-solr/pull/375
 
    {quote}
    When PrefixCompletionQuery has an empty term, its automaton is actually * -- maybe we could look into adding a fix/ optimization there in a follow-up.
    {quote}
   
    Regexp query should handle that case fine though. It uses Operations.isEmpty (see the CompiledAutomaton class) to detect that the automation accepts no strings. The logic here is different and a subset of that, and I think thats why there might be such cases.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] lucene-solr pull request #375: LUCENE-8287: Ensure that empty regex completi...

Bruno86
In reply to this post by Bruno86
Github user jtibshirani closed the pull request at:

    https://github.com/apache/lucene-solr/pull/375


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]