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/02 08:46:28 UTC
[GitHub] [lucene] LuXugang opened a new pull request, #859: LUCENE-10552: KnnVectorQuery has incorrect equals/ hashCode
LuXugang opened a new pull request, #859:
URL: https://github.com/apache/lucene/pull/859
filter query could impact document-filtering properties, so maybe it should be a condition in Query#equals and Query#hashCode ?
--
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: [GitHub] [lucene] jpountz commented on pull request #859: LUCENE-10552: KnnVectorQuery has incorrect equals/ hashCode
Posted by Michael Sokolov <ms...@gmail.com>.
+1 to back port. It will make things more consistent at least
On Thu, May 12, 2022, 11:36 AM GitBox <gi...@apache.org> wrote:
>
> jpountz commented on PR #859:
> URL: https://github.com/apache/lucene/pull/859#issuecomment-1125144256
>
> FWIW I found about this PR because it is in the 9.2 changelog on `main`
> but not on `branch_9x`. We could fix the changelog, but it looks like it's
> as easy to backport the change to `branch_9x`, and it looks safe?
>
>
> --
> 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] msokolov commented on a diff in pull request #859: LUCENE-10552: KnnVectorQuery has incorrect equals/ hashCode
Posted by GitBox <gi...@apache.org>.
msokolov commented on code in PR #859:
URL: https://github.com/apache/lucene/pull/859#discussion_r863048001
##########
lucene/core/src/test/org/apache/lucene/search/TestKnnVectorQuery.java:
##########
@@ -56,7 +56,6 @@ public void testEquals() {
Query filter1 = new TermQuery(new Term("id", "id1"));
KnnVectorQuery q2 = new KnnVectorQuery("f1", new float[] {0, 1}, 10, filter1);
- assertNotEquals(q2, q1);
Review Comment:
Nit: I think we do want to assert that equals is reflexive?
--
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] msokolov commented on pull request #859: LUCENE-10552: KnnVectorQuery has incorrect equals/ hashCode
Posted by GitBox <gi...@apache.org>.
msokolov commented on PR #859:
URL: https://github.com/apache/lucene/pull/859#issuecomment-1115412496
Oh, I changed the commit message to be positive about what the changes is, rather than describing the problem, but did not update the CHANGES. I guess it's minor
--
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] jtibshirani commented on pull request #859: LUCENE-10552: KnnVectorQuery has incorrect equals/ hashCode
Posted by GitBox <gi...@apache.org>.
jtibshirani commented on PR #859:
URL: https://github.com/apache/lucene/pull/859#issuecomment-1125422896
I just backported this to `branch_9x` to match the changelog/ fix version in JIRA. It went smoothly and seems low risk.
--
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] msokolov merged pull request #859: LUCENE-10552: KnnVectorQuery has incorrect equals/ hashCode
Posted by GitBox <gi...@apache.org>.
msokolov merged PR #859:
URL: https://github.com/apache/lucene/pull/859
--
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] msokolov commented on a diff in pull request #859: LUCENE-10552: KnnVectorQuery has incorrect equals/ hashCode
Posted by GitBox <gi...@apache.org>.
msokolov commented on code in PR #859:
URL: https://github.com/apache/lucene/pull/859#discussion_r862982712
##########
lucene/core/src/java/org/apache/lucene/search/KnnVectorQuery.java:
##########
@@ -288,15 +288,20 @@ public void visit(QueryVisitor visitor) {
@Override
public boolean equals(Object obj) {
- return sameClassAs(obj)
- && ((KnnVectorQuery) obj).k == k
- && ((KnnVectorQuery) obj).field.equals(field)
- && Arrays.equals(((KnnVectorQuery) obj).target, target);
+ if (sameClassAs(obj) == false) {
+ return false;
+ }
+ return ((KnnVectorQuery) obj).k == k
+ && ((KnnVectorQuery) obj).field.equals(field)
+ && Arrays.equals(((KnnVectorQuery) obj).target, target)
+ && ((KnnVectorQuery) obj).filter == null
+ ? filter == null
Review Comment:
How about using Objects.equals(filter, ((KnnVectorQuery) obj).filter)? This will handle the null==null case
--
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] LuXugang commented on a diff in pull request #859: LUCENE-10552: KnnVectorQuery has incorrect equals/ hashCode
Posted by GitBox <gi...@apache.org>.
LuXugang commented on code in PR #859:
URL: https://github.com/apache/lucene/pull/859#discussion_r863001028
##########
lucene/core/src/java/org/apache/lucene/search/KnnVectorQuery.java:
##########
@@ -288,15 +288,20 @@ public void visit(QueryVisitor visitor) {
@Override
public boolean equals(Object obj) {
- return sameClassAs(obj)
- && ((KnnVectorQuery) obj).k == k
- && ((KnnVectorQuery) obj).field.equals(field)
- && Arrays.equals(((KnnVectorQuery) obj).target, target);
+ if (sameClassAs(obj) == false) {
+ return false;
+ }
+ return ((KnnVectorQuery) obj).k == k
+ && ((KnnVectorQuery) obj).field.equals(field)
+ && Arrays.equals(((KnnVectorQuery) obj).target, target)
+ && ((KnnVectorQuery) obj).filter == null
+ ? filter == null
Review Comment:
Thanks @msokolov , addressd in https://github.com/apache/lucene/pull/859/commits/abe27989ef2fc650191bdbd916ee0a48dd8e2d41 .
--
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] jpountz commented on pull request #859: LUCENE-10552: KnnVectorQuery has incorrect equals/ hashCode
Posted by GitBox <gi...@apache.org>.
jpountz commented on PR #859:
URL: https://github.com/apache/lucene/pull/859#issuecomment-1125144256
FWIW I found about this PR because it is in the 9.2 changelog on `main` but not on `branch_9x`. We could fix the changelog, but it looks like it's as easy to backport the change to `branch_9x`, and it looks safe?
--
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] jpountz commented on pull request #859: LUCENE-10552: KnnVectorQuery has incorrect equals/ hashCode
Posted by GitBox <gi...@apache.org>.
jpountz commented on PR #859:
URL: https://github.com/apache/lucene/pull/859#issuecomment-1124695511
@msokolov Should this be backported to branch_9x?
--
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] msokolov commented on pull request #859: LUCENE-10552: KnnVectorQuery has incorrect equals/ hashCode
Posted by GitBox <gi...@apache.org>.
msokolov commented on PR #859:
URL: https://github.com/apache/lucene/pull/859#issuecomment-1125113064
I think it's really not significant since this query will never be cached? @jtibshirani pointed out that the filter gets applied during `rewrite()`
--
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