You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2021/02/05 19:35:09 UTC
[GitHub] [lucene-solr] jtibshirani opened a new pull request #2310: LUCENE-9705: Create Lucene90PostingsFormat
jtibshirani opened a new pull request #2310:
URL: https://github.com/apache/lucene-solr/pull/2310
For now this is just a copy of Lucene90PostingsFormat. The existing
Lucene84PostingsFormat was moved to backwards-codecs, along with its utility
classes.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] jtibshirani merged pull request #2310: LUCENE-9705: Create Lucene90PostingsFormat
Posted by GitBox <gi...@apache.org>.
jtibshirani merged pull request #2310:
URL: https://github.com/apache/lucene-solr/pull/2310
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] jtibshirani edited a comment on pull request #2310: LUCENE-9705: Create Lucene90PostingsFormat
Posted by GitBox <gi...@apache.org>.
jtibshirani edited a comment on pull request #2310:
URL: https://github.com/apache/lucene-solr/pull/2310#issuecomment-777746181
Moving these classes under versioned packages like `org.apache.lucene.codecs.lucene90` makes sense to me.
I slightly prefer the name `Lucene90BlockTreeTermsReader` because it isn't always clear where to stick the version number. But no strong opinion, and I see we already have some classes like `Completion84PostingsFormat`. Also maybe we'll omit the number in smaller helper classes like `ForDeltaUtil`.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] jpountz commented on pull request #2310: LUCENE-9705: Create Lucene90PostingsFormat
Posted by GitBox <gi...@apache.org>.
jpountz commented on pull request #2310:
URL: https://github.com/apache/lucene-solr/pull/2310#issuecomment-779931054
These names work for me too. Agreed we don't have to have numbers in small helper classes.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] iverase commented on pull request #2310: LUCENE-9705: Create Lucene90PostingsFormat
Posted by GitBox <gi...@apache.org>.
iverase commented on pull request #2310:
URL: https://github.com/apache/lucene-solr/pull/2310#issuecomment-777438974
I have a look and I have two thoughts:
1) I wonder if we should not version PFUtil classes, instead move then to a package under Util and change the visibility of the methods. Those classes seems more like a utility to me.
2) Under the codecs module, there are plenty of specialise posting readers, they all have hardcodes the posting format version. I wonder if we can expose through the posting format the creation of `PostingsReaderBase`? From the SegmentReadState we should be able to get the current codec.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] iverase edited a comment on pull request #2310: LUCENE-9705: Create Lucene90PostingsFormat
Posted by GitBox <gi...@apache.org>.
iverase edited a comment on pull request #2310:
URL: https://github.com/apache/lucene-solr/pull/2310#issuecomment-777438974
I have a look and I have two thoughts:
1) I wonder if we should not version PFUtil classes, instead move then to a package under Util and change the visibility of the methods. Those classes seems more like a utility to me.
~~2) Under the codecs module, there are plenty of specialise posting readers, they all have hardcodes the posting format version. I wonder if we can expose through the posting format the creation of `PostingsReaderBase`? From the SegmentReadState we should be able to get the current codec.~~
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] jpountz commented on pull request #2310: LUCENE-9705: Create Lucene90PostingsFormat
Posted by GitBox <gi...@apache.org>.
jpountz commented on pull request #2310:
URL: https://github.com/apache/lucene-solr/pull/2310#issuecomment-777690621
> Any thoughts on naming or package structure for these classes?
What about putting the current blocktree classes into the `org.apache.lucene.backward_codecs.lucene40` (since they were introduced in Lucene 4.0) package and renaming the reader/writer classes to include the version too, ie. `BlockTree40TermsWriter` and `BlockTree40TermsReader`? And the new classes would be called `BlockTree90TermsWriter` and `BlockTree90TermsReader` and be under the `org.apache.lucene.codecs.lucene90` package?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] jtibshirani commented on pull request #2310: LUCENE-9705: Create Lucene90PostingsFormat
Posted by GitBox <gi...@apache.org>.
jtibshirani commented on pull request #2310:
URL: https://github.com/apache/lucene-solr/pull/2310#issuecomment-780194250
@jpountz @iverase I pushed some changes, would you be able to take one last look?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] jtibshirani commented on pull request #2310: LUCENE-9705: Create Lucene90PostingsFormat
Posted by GitBox <gi...@apache.org>.
jtibshirani commented on pull request #2310:
URL: https://github.com/apache/lucene-solr/pull/2310#issuecomment-777746181
Moving these classes under versioned packages like `org.apache.lucene.codecs.lucene90` makes sense to me. I slightly prefer the name `Lucene90BlockTreeTermsReader` because it isn't always clear where to stick the version number. But no strong opinion, and I see we already have some classes like `Completion84PostingsFormat`.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] jtibshirani commented on pull request #2310: LUCENE-9705: Create Lucene90PostingsFormat
Posted by GitBox <gi...@apache.org>.
jtibshirani commented on pull request #2310:
URL: https://github.com/apache/lucene-solr/pull/2310#issuecomment-774254300
I also plan to copy over the blocktree dependencies like `BlockTreeTermsWriter`. Any thoughts on naming or package structure for these classes? In [LUCENE-9616](https://issues.apache.org/jira/browse/LUCENE-9616) @jpountz mentioned adding version numbers to the names.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene-solr] jpountz commented on pull request #2310: LUCENE-9705: Create Lucene90PostingsFormat
Posted by GitBox <gi...@apache.org>.
jpountz commented on pull request #2310:
URL: https://github.com/apache/lucene-solr/pull/2310#issuecomment-777686222
> I wonder if we should not version PFUtil classes, instead move then to a package under Util and change the visibility of the methods. Those classes seems more like a utility to me.
I've become a bit wary of having shared utility classes for codecs given how it makes the code harder to evolve (e.g. I have the FST and PackedInts classes in mind). I'd rather like to copy this utility class wherever it's needed so that every file format that uses bit packing can more easily update the logic to fits its own needs.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org