[
https://issues.apache.org/jira/browse/LUCENE-6865?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14984702#comment-14984702
]
Trejkaz edited comment on LUCENE-6865 at 11/2/15 3:44 AM:
----------------------------------------------------------
Temporary workaround for now I guess. Override the process method to wedge in
an extra call to a method which corrects the problem.
{code}
@Override
public QueryNode process(QueryNode queryTree) throws QueryNodeException
{
for (QueryNodeProcessor processor : this)
{
queryTree = processor.process(queryTree);
fixQueryTree(queryTree);
}
return queryTree;
}
/**
* Recursively fixes parent-child links in the query tree.
* Basically this works by detaching and reattaching all descendants from
their immediate parent.
*
* @param input the input query node.
*/
private void fixQueryTree(QueryNode input)
{
List<QueryNode> children = input.getChildren();
if (children != null)
{
children.forEach(this::fixQueryTree);
// This is what fixes it.
input.set(children);
}
}
{code}
was (Author: trejkaz):
Temporary workaround for now I guess. Override the process method to wedge in
an extra call to a method which corrects the problem.
{code}
@Override
public QueryNode process(QueryNode queryTree) throws QueryNodeException
{
// Reimplementing the method from the superclass so that we can do
additional checking.
for (QueryNodeProcessor processor : this)
{
queryTree = processor.process(queryTree);
fixQueryTree(queryTree);
}
return queryTree;
}
/**
* Recursively fixes parent-child links in the query tree.
* Basically this works by detaching and reattaching all descendants from
their immediate parent.
*
* @param input the input query node.
*/
private void fixQueryTree(QueryNode input)
{
List<QueryNode> children = input.getChildren();
if (children != null)
{
children.forEach(this::fixQueryTree);
// This is what fixes it.
input.set(children);
}
}
{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: [email protected]
For additional commands, e-mail: [email protected]
