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 2022/05/05 09:32:31 UTC
[GitHub] [lucene] romseygeek opened a new pull request, #869: LUCENE-10436: Reinstate public getdocValuesdocIdSetIterator method on DocValues
romseygeek opened a new pull request, #869:
URL: https://github.com/apache/lucene/pull/869
When this was refactored previously, we moved a public static method from
DocValuesFieldExistsQuery to the package-private DocValuesIterator class. This
makes the method available again by moving it instead to the public DocValues
utility class.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
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] romseygeek commented on pull request #869: LUCENE-10436: Reinstate public getdocValuesdocIdSetIterator method on DocValues
Posted by GitBox <gi...@apache.org>.
romseygeek commented on PR #869:
URL: https://github.com/apache/lucene/pull/869#issuecomment-1118516530
> Please, let's make it package private somewhere else.
It already is package private, but it was public before, and we use it in elasticsearch code. I'm happy to put it elsewhere (on FieldExistsQuery maybe?) but I don't think we can just remove public methods
--
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.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
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] rmuir commented on pull request #869: LUCENE-10436: Reinstate public getdocValuesdocIdSetIterator method on DocValues
Posted by GitBox <gi...@apache.org>.
rmuir commented on PR #869:
URL: https://github.com/apache/lucene/pull/869#issuecomment-1118511195
Why in the world are we moving a method to DocValues API that is only used by 3 callsites. Please, let's make it package private somewhere else.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
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] romseygeek commented on pull request #869: LUCENE-10436: Reinstate public getdocValuesdocIdSetIterator method on DocValues
Posted by GitBox <gi...@apache.org>.
romseygeek commented on PR #869:
URL: https://github.com/apache/lucene/pull/869#issuecomment-1118588920
That I agree with! I'll update and put it on FieldExistsQuery - it was on DocValuesFieldExistsQuery before, which has been deprecated but now extends FEQ, and so it should make the transition a lot easier.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
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] rmuir commented on pull request #869: LUCENE-10436: Reinstate public getdocValuesdocIdSetIterator method on DocValues
Posted by GitBox <gi...@apache.org>.
rmuir commented on PR #869:
URL: https://github.com/apache/lucene/pull/869#issuecomment-1118557005
I'm not so opposed to the method being public somewhere, I'm more questioning the need to put it in `DocValues` api. This is what grabbed my attention. Would love to keep this API simple and minimal and without exotic stuff. Today the methods it uses are type-safe and here we are adding a relatively "untyped" method to get a generic iterator over any DV type. If you look at the other methods in the file, it really doesn't fit.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
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] rmuir commented on pull request #869: LUCENE-10436: Reinstate public getdocValuesdocIdSetIterator method on DocValues
Posted by GitBox <gi...@apache.org>.
rmuir commented on PR #869:
URL: https://github.com/apache/lucene/pull/869#issuecomment-1118526833
> It already is package private, but it was public before, and we use it in elasticsearch code. I'm happy to put it elsewhere (on FieldExistsQuery maybe?) but I don't think we can just remove public methods
Sure we can. used by elasticsearch doesn't mean its a requirement to be public. Sorry, this is just a bit of a pain point as the most recent 2 pull requests in lucene are API changes just like this: for elasticsearch and amazon respectively.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
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] romseygeek commented on pull request #869: LUCENE-10436: Reinstate public getdocValuesdocIdSetIterator method on DocValues
Posted by GitBox <gi...@apache.org>.
romseygeek commented on PR #869:
URL: https://github.com/apache/lucene/pull/869#issuecomment-1118537137
I'd argue that it's a revert of an API change - it's a public method in 9.1 and currently we're removing it in 9.2 with no CHANGES entry or information about how to migrate. And the fact that we're using it in ES suggests that there may be other users of it as well. If we really think it shouldn't be a public method, fine, but we should at least have some information on how consumers who are using it at the moment should handle upgrades?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
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] romseygeek commented on pull request #869: LUCENE-10436: Reinstate public getdocValuesdocIdSetIterator method on DocValues
Posted by GitBox <gi...@apache.org>.
romseygeek commented on PR #869:
URL: https://github.com/apache/lucene/pull/869#issuecomment-1118357618
Having said that, looking at the existing MIGRATE instructions, maybe the most sensible thing is to have this method on FieldExistsQuery directly? Then existing code that references the deprecated class continues to work in 9x, and its a fairly simple search-and-replace for upgrading?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
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] romseygeek commented on pull request #869: LUCENE-10436: Reinstate public getdocValuesdocIdSetIterator method on DocValues
Posted by GitBox <gi...@apache.org>.
romseygeek commented on PR #869:
URL: https://github.com/apache/lucene/pull/869#issuecomment-1118356099
When I backport I'll add a deprecated forwarding method to DocValuesFieldExistsQuery again, to make it a bit more obvious on how to migrate.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
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] romseygeek merged pull request #869: LUCENE-10436: Reinstate public getdocValuesdocIdSetIterator method on DocValues
Posted by GitBox <gi...@apache.org>.
romseygeek merged PR #869:
URL: https://github.com/apache/lucene/pull/869
--
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.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
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