dsmiley commented on a change in pull request #1970:
URL: https://github.com/apache/lucene-solr/pull/1970#discussion_r502745875
##########
File path:
solr/core/src/test/org/apache/solr/response/transform/TestChildDocTransformerHierarchy.java
##########
@@ -135,6 +135,37 @@ public void testParentFilterLimitJSON() throws Exception {
);
}
+ @Test
+ public void testLiveDocs() throws Exception {
Review comment:
```suggestion
public void testWithDeletedChildren() throws Exception {
```
"liveDocs" is Lucene level internal terminology.
##########
File path:
solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformer.java
##########
@@ -126,6 +128,12 @@ public void transform(SolrDocument rootDoc, int rootDocId)
{
continue;
}
+ // check whether doc is "live"
Review comment:
Maybe do this at the top of the loop because it's cheap?
##########
File path:
solr/core/src/java/org/apache/solr/response/transform/ChildDocTransformer.java
##########
@@ -126,6 +128,12 @@ public void transform(SolrDocument rootDoc, int rootDocId)
{
continue;
}
+ // check whether doc is "live"
+ if (liveDocs != null && !liveDocs.get(docId)) {
Review comment:
```suggestion
if (liveDocs != null && !liveDocs.get(docId - segBaseId)) {
```
See if your test could have caught this by adding a doc & committing up
front, thus creating an additional segment? You could wrap in a
random().nextBoolean()
----------------------------------------------------------------
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:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
