You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucenenet.apache.org by GitBox <gi...@apache.org> on 2020/09/30 12:44:32 UTC
[GitHub] [lucenenet] amartens181 opened a new issue #360: AnalyzingInfixSuggester Index Storage
amartens181 opened a new issue #360:
URL: https://github.com/apache/lucenenet/issues/360
### Expected Outcome:
There would be a way to persist an index built by the suggester.
### What I See:
The index is never commited, so the index created is one-time use. I had been using `RamDirectory` to store it since I was not indexing much, but now I want to persist it now.
### How To Recreate:
Build an `AnalyzingInfixSuggester`. Without using reflection, there's no way of finalizing the index to disk.
### Other Info:
The ability to commit the index was added in 4.11.0, but the inability to do so in 4.8.0 without reflection seems like a usability bug.
Java commit code: https://github.com/apache/lucene-solr/commit/4a485d68771e5e123139cc12e9ea2733a922baea
I'm using reflection to commit it after I build:
```csharp
suggester.Build(enumerator);
var writer = suggester.GetType().GetField("writer", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance);
typeof(IndexWriter).GetMethod("Commit").Invoke(writer.GetValue(suggester), Array.Empty<object>());
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [lucenenet] NightOwl888 commented on issue #360: AnalyzingInfixSuggester Index Storage
Posted by GitBox <gi...@apache.org>.
NightOwl888 commented on issue #360:
URL: https://github.com/apache/lucenenet/issues/360#issuecomment-768126835
Note that the Lucene JIRA issue that discusses this fix is [LUCENE-5889](https://issues.apache.org/jira/browse/LUCENE-5889).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [lucenenet] rclabo commented on issue #360: AnalyzingInfixSuggester Index Storage
Posted by GitBox <gi...@apache.org>.
rclabo commented on issue #360:
URL: https://github.com/apache/lucenenet/issues/360#issuecomment-768298151
It's too bad the member var for IndexWriter in the Java class is private rather then protected. So inheriting from `AnalyzingInfixSuggester` and adding on a simple method
```
public virtual void Commit() {
writer.Commit();
}
```
isn't an option sadly. Makes a person appreciate the power of making member vars protected rather than private. Something I don't do in my own code nearly enough.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [lucenenet] rclabo commented on issue #360: AnalyzingInfixSuggester Index Storage
Posted by GitBox <gi...@apache.org>.
rclabo commented on issue #360:
URL: https://github.com/apache/lucenenet/issues/360#issuecomment-769191728
In the diffs for the LUCENE-5889 [commit](https://github.com/apache/lucene-solr/commit/4a485d68771e5e123139cc12e9ea2733a922baea) There are two files that are part of solr rather than Lucene. These two files appear as the last two in the diff and are:
solr/core/src/java/org/apache/solr/spelling/suggest/fst/AnalyzingInfixLookupFactory.java
solr/core/src/java/org/apache/solr/spelling/suggest/fst/BlendedInfixLookupFactory.java
Since they are part of solr rather than Lucene they do not currently exist in the Lucene.Net repository. I'm guessing they they should not be added as part of my implementation of LUCENE-5889. Agreed?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [lucenenet] rclabo commented on issue #360: AnalyzingInfixSuggester Index Storage
Posted by GitBox <gi...@apache.org>.
rclabo commented on issue #360:
URL: https://github.com/apache/lucenenet/issues/360#issuecomment-768412488
I'd be happy to make the needed change and submit it. This should be a nice first issue for me. I would have assigned this issue to myself but didn't see how (the check box was missing from the list of issues view).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [lucenenet] NightOwl888 closed issue #360: AnalyzingInfixSuggester Index Storage
Posted by GitBox <gi...@apache.org>.
NightOwl888 closed issue #360:
URL: https://github.com/apache/lucenenet/issues/360
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [lucenenet] NightOwl888 commented on issue #360: AnalyzingInfixSuggester Index Storage
Posted by GitBox <gi...@apache.org>.
NightOwl888 commented on issue #360:
URL: https://github.com/apache/lucenenet/issues/360#issuecomment-703180922
@amartens181
While strictly speaking we are supposed to leave the limitations in place of 4.8.0, we are making some exceptions in places where usability is a factor, especially in cases where there is no reasonable way to use inheritance to fix the issue. In this case, I would approve the changes provided the tests all pass and there are no ill effects produced by adding the patch out of order.
That being said, we are trying to make as straight of a line as possible to the 4.8.0 release. As such, these "extra" changes are something we probably won't make the extra effort to do. However, we will include them if a PR is submitted by the community.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [lucenenet] NightOwl888 commented on issue #360: AnalyzingInfixSuggester Index Storage
Posted by GitBox <gi...@apache.org>.
NightOwl888 commented on issue #360:
URL: https://github.com/apache/lucenenet/issues/360#issuecomment-770104703
@rclabo
Thanks for the PR. This feature is now implemented and will be in the next beta release.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [lucenenet] NightOwl888 commented on issue #360: AnalyzingInfixSuggester Index Storage
Posted by GitBox <gi...@apache.org>.
NightOwl888 commented on issue #360:
URL: https://github.com/apache/lucenenet/issues/360#issuecomment-768452713
@rclabo
Sure, this is something that both looks easy and might not be as easy as it looks :grin:.
Please make sure it is clear (with either comments or c# sections) which parts are ported from after Lucene 4.8.0. By convention, we use `\\ LUCENENET specific - ` followed by a brief note about how we have diverged from Lucene 4.8.0.
Those extra constructor parameters look innocuous enough until you think about the fact that it is a breaking constructor API change. Please add another ctor overload while preserving both the signature and behavior of the original 4.8.0 constructor, just to make sure the changes are completely compatible with 4.8.0-ported code.
> Note: Microsoft almost never uses optional parameters in their code, so that is something we usually try to avoid.
By the same measure, tests should be set up to ensure exact 4.8.0 compatibility as well as for testing the complete functionality of these new changes, and we should ensure that these 2 groups of tests are clearly delineated in the code with a clear path forward when the code is upgraded beyond 4.8.0.
Fortunately, adding a `Commit()` method is not a breaking API change, so there is no issue with simply adding it on top of the existing API.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [lucenenet] NightOwl888 commented on issue #360: AnalyzingInfixSuggester Index Storage
Posted by GitBox <gi...@apache.org>.
NightOwl888 commented on issue #360:
URL: https://github.com/apache/lucenenet/issues/360#issuecomment-768503501
Much like https://github.com/apache/lucenenet/issues/274, it is generally easier to keep track of changes if an entire Lucene patch is applied, rather than pulling out individual changes within the patch. This isn't always completely possible due to the fact that some patches depend on changes that exist in other patches, but it is a bit easier to reconcile the state of the release if we know what patches have been added.
- Lucene 4.8.0
- \+ LUCENE-6001
- \+ LUCENE-5889
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [lucenenet] NightOwl888 commented on issue #360: AnalyzingInfixSuggester Index Storage
Posted by GitBox <gi...@apache.org>.
NightOwl888 commented on issue #360:
URL: https://github.com/apache/lucenenet/issues/360#issuecomment-768454957
Also note that in [LUCENE-5889](https://issues.apache.org/jira/browse/LUCENE-5889) these changes extend beyond just the `AnalyzingInfixSuggester` class, as the functionality applies to some other suggesters as well.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [lucenenet] NightOwl888 commented on issue #360: AnalyzingInfixSuggester Index Storage
Posted by GitBox <gi...@apache.org>.
NightOwl888 commented on issue #360:
URL: https://github.com/apache/lucenenet/issues/360#issuecomment-769210572
Yes, that is correct. We have no plans to port SOLR, although we might consider that down the road when upgrading to a more recent Lucene version.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [lucenenet] rclabo commented on issue #360: AnalyzingInfixSuggester Index Storage
Posted by GitBox <gi...@apache.org>.
rclabo commented on issue #360:
URL: https://github.com/apache/lucenenet/issues/360#issuecomment-768486539
I was initially just proposing to add the `commit` method to the `AnalyzingInfixSuggester` because I figured you wanted to keep Lucene.Net 4.8 as close to the java version as possible. But I'd be happy to fully apply [LUCENE-5889](https://issues.apache.org/jira/browse/LUCENE-5889) . That will be a step of from the trivial change I intended but still seems fairly easy I think. Will do.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org