From cc6c07365c89e1608b78a46af7744a67ff5277a1 Mon Sep 17 00:00:00 2001 From: Shay Banon Date: Fri, 8 Mar 2013 08:50:11 -0800 Subject: [PATCH] has_child query AVG score mode does not always work correctly fixes #2750 --- .../index/search/child/ChildrenQuery.java | 30 +++++++++++++++---- .../index/search/child/ParentQuery.java | 5 ++-- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/elasticsearch/index/search/child/ChildrenQuery.java b/src/main/java/org/elasticsearch/index/search/child/ChildrenQuery.java index 71cd01f2852..58527e95d41 100644 --- a/src/main/java/org/elasticsearch/index/search/child/ChildrenQuery.java +++ b/src/main/java/org/elasticsearch/index/search/child/ChildrenQuery.java @@ -48,6 +48,7 @@ import java.util.Set; * all parent documents having the same uid value that is collected in the first phase are emitted as hit including * a score based on the aggregated child scores and score type. */ +// TODO We use a score of 0 to indicate a doc was not scored in uidToScore, this means score of 0 can be problematic, if we move to HPCC, we can use lset/... public class ChildrenQuery extends Query implements SearchContext.Rewrite { private final SearchContext searchContext; @@ -250,7 +251,7 @@ public class ChildrenQuery extends Query implements SearchContext.Rewrite { HashedBytesArray uid = idTypeCache.idByDoc(currentDocId); currentScore = uidToScore.get(uid); - if (Float.compare(currentScore, 0) > 0) { + if (currentScore != 0) { return currentDocId; } } @@ -265,7 +266,7 @@ public class ChildrenQuery extends Query implements SearchContext.Rewrite { HashedBytesArray uid = idTypeCache.idByDoc(currentDocId); currentScore = uidToScore.get(uid); - if (Float.compare(currentScore, 0) > 0) { + if (currentScore != 0) { return currentDocId; } else { return nextDoc(); @@ -293,12 +294,29 @@ public class ChildrenQuery extends Query implements SearchContext.Rewrite { currentUid = idTypeCache.idByDoc(currentDocId); currentScore = uidToScore.get(currentUid); - if (Float.compare(currentScore, 0) > 0) { + if (currentScore != 0) { currentScore /= uidToCount.get(currentUid); return currentDocId; } } } + + @Override + public int advance(int target) throws IOException { + currentDocId = parentsIterator.advance(target); + if (currentDocId == DocIdSetIterator.NO_MORE_DOCS) { + return currentDocId; + } + + HashedBytesArray uid = idTypeCache.idByDoc(currentDocId); + currentScore = uidToScore.get(uid); + if (currentScore != 0) { + currentScore /= uidToCount.get(currentUid); + return currentDocId; + } else { + return nextDoc(); + } + } } static class ChildUidCollector extends NoopCollector { @@ -327,7 +345,7 @@ public class ChildrenQuery extends Query implements SearchContext.Rewrite { HashedBytesArray parentUid = typeCache.parentIdByDoc(doc); float previousScore = uidToScore.get(parentUid); float currentScore = scorer.score(); - if (Float.compare(previousScore, 0) == 0) { + if (previousScore == 0) { uidToScore.put(parentUid, currentScore); } else { switch (scoreType) { @@ -335,7 +353,7 @@ public class ChildrenQuery extends Query implements SearchContext.Rewrite { uidToScore.adjustValue(parentUid, currentScore); break; case MAX: - if (Float.compare(previousScore, currentScore) < 0) { + if (currentScore > previousScore) { uidToScore.put(parentUid, currentScore); } break; @@ -374,7 +392,7 @@ public class ChildrenQuery extends Query implements SearchContext.Rewrite { HashedBytesArray parentUid = typeCache.parentIdByDoc(doc); float previousScore = uidToScore.get(parentUid); float currentScore = scorer.score(); - if (Float.compare(previousScore, 0) == 0) { + if (previousScore == 0) { uidToScore.put(parentUid, currentScore); uidToCount.put(parentUid, 1); } else { diff --git a/src/main/java/org/elasticsearch/index/search/child/ParentQuery.java b/src/main/java/org/elasticsearch/index/search/child/ParentQuery.java index 715a748cec9..f1eb3b3b364 100644 --- a/src/main/java/org/elasticsearch/index/search/child/ParentQuery.java +++ b/src/main/java/org/elasticsearch/index/search/child/ParentQuery.java @@ -43,6 +43,7 @@ import java.util.Set; * connects the matching parent docs to the related child documents * using the {@link IdReaderTypeCache}. */ +// TODO We use a score of 0 to indicate a doc was not scored in uidToScore, this means score of 0 can be problematic, if we move to HPCC, we can use lset/... public class ParentQuery extends Query implements SearchContext.Rewrite { private final SearchContext searchContext; @@ -258,7 +259,7 @@ public class ParentQuery extends Query implements SearchContext.Rewrite { continue; } currentScore = uidToScore.get(uid); - if (Float.compare(currentScore, 0) != 0) { + if (currentScore != 0) { return currentChildDoc; } } @@ -275,7 +276,7 @@ public class ParentQuery extends Query implements SearchContext.Rewrite { return nextDoc(); } currentScore = uidToScore.get(uid); - if (Float.compare(currentScore, 0) == 0) { + if (currentScore == 0) { return nextDoc(); } return currentChildDoc;