[
https://issues.apache.org/jira/browse/LUCENE-6865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14979528#comment-14979528
]
Trejkaz commented on LUCENE-6865:
---------------------------------
Standalone reproduction:
{code}
import java.util.Arrays;
import java.util.List;
import org.apache.lucene.queryparser.flexible.core.nodes.AndQueryNode;
import org.apache.lucene.queryparser.flexible.core.nodes.GroupQueryNode;
import org.apache.lucene.queryparser.flexible.core.nodes.OrQueryNode;
import org.apache.lucene.queryparser.flexible.core.nodes.QueryNode;
import org.apache.lucene.queryparser.flexible.core.nodes.QuotedFieldQueryNode;
import
org.apache.lucene.queryparser.flexible.core.processors.QueryNodeProcessor;
import
org.apache.lucene.queryparser.flexible.standard.config.StandardQueryConfigHandler;
import
org.apache.lucene.queryparser.flexible.standard.processors.BooleanQuery2ModifierNodeProcessor;
import org.junit.Test;
import static org.hamcrest.Matchers.*;
import static org.junit.Assert.*;
/**
* Tests for {@link BooleanQuery2ModifierNodeProcessor},
*/
public class TestBooleanQuery2ModifierNodeProcessor
{
@Test
public void testProcess() throws Exception
{
QueryNode before = new GroupQueryNode(
new AndQueryNode(Arrays.asList(
new GroupQueryNode(
new OrQueryNode(Arrays.asList(
new QuotedFieldQueryNode("a", "a", 0, 0),
new QuotedFieldQueryNode("a", "a", 0, 0)))),
new GroupQueryNode(
new OrQueryNode(Arrays.asList(
new QuotedFieldQueryNode("a", "a", 0, 0),
new QuotedFieldQueryNode("a", "a", 0, 0)))))));
QueryNodeProcessor processor = new BooleanQuery2ModifierNodeProcessor();
processor.setQueryConfigHandler(new StandardQueryConfigHandler());
QueryNode after = processor.process(before);
checkTree(after);
}
private static void checkTree(QueryNode queryNode)
{
checkTree(queryNode, "");
}
private static void checkTree(QueryNode queryNode, String prefix)
{
List<QueryNode> children = queryNode.getChildren();
if (children != null)
{
String childPrefix = " " + prefix;
for (QueryNode child : children)
{
assertThat(child.getParent(), is(queryNode));
checkTree(child, childPrefix);
}
}
}
}
{code}
> BooleanQuery2ModifierNodeProcessor breaks the query node hierarchy
> ------------------------------------------------------------------
>
> Key: LUCENE-6865
> URL: https://issues.apache.org/jira/browse/LUCENE-6865
> Project: Lucene - Core
> Issue Type: Bug
> Reporter: Trejkaz
>
> We discovered that one of our own implementations of QueryNodeProcessor was
> seeing node.getParent() returning null for nodes other than the root of the
> query tree.
> I put a diagnostic processor around every processor which runs and found that
> BooleanQuery2ModifierNodeProcessor (and possibly others, although it isn't
> clear) are mysteriously setting some of the node references to null.
> Example query tree before:
> {noformat}
> GroupQueryNode, parent = null
> WithinQueryNode, parent = GroupQueryNode
> QuotedFieldQueryNode, parent = WithinQueryNode
> GroupQueryNode, parent = WithinQueryNode
> AndQueryNode, parent = GroupQueryNode
> GroupQueryNode, parent = AndQueryNode
> OrQueryNode, parent = GroupQueryNode
> QuotedFieldQueryNode, parent = OrQueryNode
> QuotedFieldQueryNode, parent = OrQueryNode
> GroupQueryNode, parent = AndQueryNode
> OrQueryNode, parent = GroupQueryNode
> QuotedFieldQueryNode, parent = OrQueryNode
> QuotedFieldQueryNode, parent = OrQueryNode
> {noformat}
> And after BooleanQuery2ModifierNodeProcessor.process():
> {noformat}
> GroupQueryNode, parent = null
> WithinQueryNode, parent = GroupQueryNode
> QuotedFieldQueryNode, parent = WithinQueryNode
> GroupQueryNode, parent = WithinQueryNode
> AndQueryNode, parent = GroupQueryNode
> BooleanModifierNode, parent = AndQueryNode
> GroupQueryNode, parent = null
> OrQueryNode, parent = GroupQueryNode
> QuotedFieldQueryNode, parent = OrQueryNode
> QuotedFieldQueryNode, parent = OrQueryNode
> BooleanModifierNode, parent = AndQueryNode
> GroupQueryNode, parent = null
> OrQueryNode, parent = GroupQueryNode
> QuotedFieldQueryNode, parent = OrQueryNode
> QuotedFieldQueryNode, parent = OrQueryNode
> {noformat}
> Looking at QueryNodeImpl, there is a lot of fiddly logic in there. Removing
> children can trigger setting the parent to null, but setting the parent can
> also trigger the child removing itself, so it's near impossible to figure out
> why this could be happening, but I'm closing in on it at least. My initial
> suspicion is that cloneTree() is responsible, because ironically the number
> of failures of this sort _increase_ if I try to use cloneTree to defend
> against mutability bugs.
> The fix I have come up with is to clone the whole API but making QueryNode
> immutable. This removes the ability for processors to mess with nodes that
> don't belong to them, but also obviates the need for a parent reference in
> the first place, which I think is the entire source of the problem - keeping
> the parent and child in sync correctly is obviously going to be hard, and
> indeed we find that there is at least one bug of this sort lurking in there.
> But even if we rewrite it, I figured I would report the issue so that maybe
> it can be fixed for others.
> Code to use for diagnostics:
> {code}
> import java.util.List;
> import org.apache.lucene.queryparser.flexible.core.QueryNodeException;
> import org.apache.lucene.queryparser.flexible.core.config.QueryConfigHandler;
> import org.apache.lucene.queryparser.flexible.core.nodes.QueryNode;
> import
> org.apache.lucene.queryparser.flexible.core.processors.QueryNodeProcessor;
> public class DiagnosticQueryNodeProcessor implements QueryNodeProcessor
> {
> private final QueryNodeProcessor delegate;
> public TreeFixingQueryNodeProcessor(QueryNodeProcessor delegate)
> {
> this.delegate = delegate;
> }
> @Override
> public QueryConfigHandler getQueryConfigHandler()
> {
> return delegate.getQueryConfigHandler();
> }
> @Override
> public void setQueryConfigHandler(QueryConfigHandler queryConfigHandler)
> {
> delegate.setQueryConfigHandler(queryConfigHandler);
> }
> @Override
> public QueryNode process(QueryNode queryNode) throws QueryNodeException
> {
> System.out.println("Before " + delegate.getClass().getSimpleName() +
> ".process():");
> dumpTree(queryNode);
> queryNode = delegate.process(queryNode);
> System.out.println("After " + delegate.getClass().getSimpleName() +
> ".process():");
> dumpTree(queryNode);
> return queryNode;
> }
> private void dumpTree(QueryNode queryNode)
> {
> dumpTree(queryNode, "");
> }
> private void dumpTree(QueryNode queryNode, String prefix)
> {
> System.out.println(prefix + queryNode.getClass().getSimpleName() +
> ", parent = " + (queryNode.getParent() == null ?
> "null" : queryNode.getParent().getClass().getSimpleName()));
> List<QueryNode> children = queryNode.getChildren();
> if (children != null)
> {
> String childPrefix = " " + prefix;
> for (QueryNode child : children)
> {
> dumpTree(child, childPrefix);
> }
> }
> }
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
