You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by "uschindler (via GitHub)" <gi...@apache.org> on 2024/12/17 13:23:04 UTC
[PR] This fixes immutability of clauseSets (broken by #13950) [lucene]
uschindler opened a new pull request, #14074:
URL: https://github.com/apache/lucene/pull/14074
This fixes immutability of `BooleanQuery` caused by PR #13950.
See comment: https://github.com/apache/lucene/pull/13950#pullrequestreview-2508662206
--
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
Re: [PR] This fixes immutability of clauseSets (broken by #13950) [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on PR #14074:
URL: https://github.com/apache/lucene/pull/14074#issuecomment-2548460187
P.S.: I added the `Collections.immutableCollection` only in the getter, because making `EnumMap clauseSets` does not work:
- The inner values of the map are custom classes. As we have no unmodifiable Multiset, we cant wrap it without breaking equals (If we map it as unmodifiableCollection, the resulting equals/hashCode inspects all elements and respects order.
- we can make the outer map immutable, but as this one is not exposed, I don't care. I just added another warning.
For the future: If people submit PRs about making private members which are collections public, always check if the immutability could be violated. This is a major problem in Java where collections are by default mutable!
--
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
Re: [PR] This fixes immutability of clauseSets (broken by #13950) [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler merged PR #14074:
URL: https://github.com/apache/lucene/pull/14074
--
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
Re: [PR] This fixes immutability of clauseSets (broken by #13950) [lucene]
Posted by "gsmiller (via GitHub)" <gi...@apache.org>.
gsmiller commented on PR #14074:
URL: https://github.com/apache/lucene/pull/14074#issuecomment-2549785261
Ah great catch. Thanks @uschindler!
--
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
Re: [PR] This fixes immutability of clauseSets (broken by #13950) [lucene]
Posted by "mikemccand (via GitHub)" <gi...@apache.org>.
mikemccand commented on PR #14074:
URL: https://github.com/apache/lucene/pull/14074#issuecomment-2548665493
> For the future: If people submit PRs about making private members which are collections public, always check if the immutability could be violated. This is a major problem in Java where collections are by default mutable!
+1
Is this something a static checker could catch?
--
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
Re: [PR] This fixes immutability of clauseSets (broken by #13950) [lucene]
Posted by "shubhamvishu (via GitHub)" <gi...@apache.org>.
shubhamvishu commented on PR #14074:
URL: https://github.com/apache/lucene/pull/14074#issuecomment-2558882131
Thanks for fixing this @uschindler !
--
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
Re: [PR] This fixes immutability of clauseSets (broken by #13950) [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on PR #14074:
URL: https://github.com/apache/lucene/pull/14074#issuecomment-2548529464
I cherrypicked in 10.x and 10.1 branch.
--
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
Re: [PR] This fixes immutability of clauseSets (broken by #13950) [lucene]
Posted by "uschindler (via GitHub)" <gi...@apache.org>.
uschindler commented on PR #14074:
URL: https://github.com/apache/lucene/pull/14074#issuecomment-2548724672
> > For the future: If people submit PRs about making private members which are collections public, always check if the immutability could be violated. This is a major problem in Java where collections are by default mutable!
>
> +1
>
> Is this something a static checker could catch?
I thought about this, too. But a checker like this is hard to implement. Maybe there's an option in newest version of errorprone, but I don't think it's easy: https://errorprone.info/bugpattern/Immutable -- this requires consistent use of some annotation in source code to mark all compoents immutable.
--
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