Author: ssmiweve
Date: 2008-11-25 14:49:33 +0100 (Tue, 25 Nov 2008)
New Revision: 6980
Modified:
branches/2.17/query-api/src/main/java/no/sesat/search/query/finder/ChildFinder.java
branches/2.17/query-api/src/main/java/no/sesat/search/query/parser/alt/AbstractAlternation.java
Log:
Issue SKER5016: (Strange infinite loop in VeryFastTokenEvaluator.java on a few
number of newscase person pages)
Modified:
branches/2.17/query-api/src/main/java/no/sesat/search/query/finder/ChildFinder.java
===================================================================
---
branches/2.17/query-api/src/main/java/no/sesat/search/query/finder/ChildFinder.java
2008-11-25 07:59:41 UTC (rev 6979)
+++
branches/2.17/query-api/src/main/java/no/sesat/search/query/finder/ChildFinder.java
2008-11-25 13:49:33 UTC (rev 6980)
@@ -1,4 +1,4 @@
-/* Copyright (2007) Schibsted Søk AS
+/* Copyright (2007-2008) Schibsted Søk AS
* This file is part of SESAT.
*
* SESAT is free software: you can redistribute it and/or modify
@@ -23,13 +23,23 @@
import no.sesat.search.query.parser.*;
-final class ChildFinder extends AbstractReflectionVisitor {
-
+/** Used to find if a particular clause exists underneath another.
+ *
+ * @version $Id$
+ */
+public final class ChildFinder extends AbstractReflectionVisitor {
+
private boolean found;
private Clause child = null;
+ /** Does the child clause exist any depth underneath the parent.
+ *
+ * @param parent the parent clause
+ * @param child the child clause.
+ * @return Does the child clause exist any depth underneath the parent.
+ */
public synchronized boolean childExists(final OperationClause parent,
final Clause child) {
-
+
found = false;
this.child = child;
visit(parent);
@@ -42,7 +52,7 @@
clause.getSecondClause().accept(this);
}
}
-
+
protected void visitImpl(final OperationClause clause) {
if (!found ) { // still looking
clause.getFirstClause().accept(this);
@@ -50,7 +60,7 @@
}
protected void visitImpl(final LeafClause clause) {
-
+
found = clause == child;
}
Modified:
branches/2.17/query-api/src/main/java/no/sesat/search/query/parser/alt/AbstractAlternation.java
===================================================================
---
branches/2.17/query-api/src/main/java/no/sesat/search/query/parser/alt/AbstractAlternation.java
2008-11-25 07:59:41 UTC (rev 6979)
+++
branches/2.17/query-api/src/main/java/no/sesat/search/query/parser/alt/AbstractAlternation.java
2008-11-25 13:49:33 UTC (rev 6980)
@@ -1,4 +1,4 @@
-/* Copyright (2007) Schibsted Søk AS
+/* Copyright (2007-2008) Schibsted Søk AS
* This file is part of SESAT.
*
* SESAT is free software: you can redistribute it and/or modify
@@ -31,6 +31,7 @@
import no.sesat.search.query.OperationClause;
import no.sesat.search.query.OrClause;
import no.sesat.search.query.XorClause;
+import no.sesat.search.query.finder.ChildFinder;
import org.apache.log4j.Logger;
/** Base abstraction class for any Alternation implementation.
@@ -41,43 +42,43 @@
* @version <tt>$Id$</tt>
*/
public abstract class AbstractAlternation implements Alternation{
-
- // Constants -----------------------------------------------------
-
+
+ // Constants -----------------------------------------------------
+
private static final Logger LOG =
Logger.getLogger(AbstractAlternation.class);
-
+
private static final String ERR_MULTIPLE_POSSIBLE_PARENTS = "Multiple
parents exist with same (or sub) class as ";
-
+
// Attributes ----------------------------------------------------
-
+
/**
* The context to work within.
*/
- protected final Context context;
-
+ protected final Context context;
+
// Static --------------------------------------------------------
-
-
+
+
// Constructors --------------------------------------------------
-
- /** Creates a new instance of AbstractAlternation
- * @param cxt
+
+ /** Creates a new instance of AbstractAlternation
+ * @param cxt
*/
public AbstractAlternation(final Context cxt) {
context = cxt;
}
-
+
// Public --------------------------------------------------------
-
-
+
+
// Package protected ---------------------------------------------
// Protected -----------------------------------------------------
-
+
/** will return null instead of a leafClause
- * @param clause
- * @return
+ * @param clause
+ * @return
*/
protected <T extends DoubleOperatorClause> T leftOpChild(final T clause){
@@ -86,8 +87,8 @@
}
/** return the left child, left or operation.
- * @param clause
- * @return
+ * @param clause
+ * @return
*/
protected Clause leftChild(final OperationClause clause) {
@@ -97,8 +98,8 @@
}
/** will return null instead of a leafClause
- * @param clause
- * @return
+ * @param clause
+ * @return
*/
protected <T extends DoubleOperatorClause> T rightOpChild(final T clause){
@@ -107,8 +108,8 @@
}
/** will return right child, leaf or operation.
- * @param clause
- * @return
+ * @param clause
+ * @return
*/
protected Clause rightChild(final DoubleOperatorClause clause) {
@@ -121,8 +122,8 @@
* And the child must be a descendant of the root.
* The result will also be assignable from the root argument's class.
* If there exists multiple parents all of the required class an
IllegalStateException is thrown.
- * @param child
- * @param root
+ * @param child
+ * @param root
*/
protected <T extends OperationClause> T parent(final T root, final Clause
child) {
@@ -140,9 +141,9 @@
}
/** return all parents operation clauses of the given child.
- * @param root
- * @param child
- * @return
+ * @param root
+ * @param child
+ * @return
*/
protected <T extends OperationClause> List<T> parents(final T root, final
Clause child) {
@@ -150,13 +151,14 @@
}
/** Build new DoubleOperatorClauses from newChild all the way back up to
the root.
- * XXX Only handles single splits, or one layer of variations, denoted by
the childsParentBeforeRotation argument.
- * This could be solved by using an array, specifying ancestry line,
for the argument instead.
- ** @param root
- * @param newChild
- * @param originalChild
- * @param originalParent
- * @return
+ * XXX Only handles single splits, or one layer of variations, denoted by
the originalParent argument.
+ * This could be solved by using an array, specifying ancestry line,
for the argument instead. <br/><br/>
+ * If, under root, originalParent cannot be found then root is returned
unaltered.
+ * @param root the root clause. an altered version of this will be
returned.
+ * @param newChild the new child.
+ * @param originalChild the original child.
+ * @param originalParent the original parent of the original child.
expected to be found under root.
+ * @return the root clause where the originalChild has been replaced with
the newChild.
*/
protected OperationClause replaceDescendant(
final DoubleOperatorClause root,
@@ -164,46 +166,55 @@
final DoubleOperatorClause originalChild,
final DoubleOperatorClause originalParent){
- OperationClause nC = newChild;
- OperationClause rC = originalChild;
- OperationClause rCParent = originalParent;
+ // pre-condition check: originalParent must be found under root
somewhere
+ if(new ChildFinder().childExists(root, originalParent)){
- do{
- nC = replaceOperatorClause(nC, rC, rCParent);
- for(OperationClause parent :
context.getParentFinder().getParents(root, rC)){
- if(rCParent == parent){
- rC = parent;
- rCParent = root == rCParent
- ? rCParent
- : context.getParentFinder().getParent(root,
rCParent);
- break;
+ OperationClause nC = newChild;
+ OperationClause rC = originalChild;
+ OperationClause rCParent = originalParent;
+
+ do{
+ nC = replaceOperatorClause(nC, rC, rCParent);
+ for(OperationClause parent :
context.getParentFinder().getParents(root, rC)){
+ if(rCParent == parent){
+ rC = parent;
+ rCParent = root == rCParent
+ ? rCParent
+ : context.getParentFinder().getParent(root,
rCParent);
+ break;
+ }
}
- }
- }while(root != rC);
+ }while(root != rC);
- return nC;
+ return nC;
+
+ }else{
+ LOG.error("originalParent does not live inside root\n" +
originalParent + '\n' + root);
+ // return the unaltered root
+ return root;
+ }
}
/** Replace the originalChild that exists under the originalParent will
the newChild.
- *
- * @param newChild
- * @param originalChild
- * @param originalParent
- * @return
+ *
+ * @param newChild
+ * @param originalChild
+ * @param originalParent
+ * @return
*/
protected <T extends OperationClause> T replaceOperatorClause(
final Clause newChild,
final Clause originalChild,
final T originalParent) {
-
+
final Clause leftChild = leftChild(originalParent) == originalChild
? newChild
: leftChild(originalParent);
-
+
final Clause rightChild;
-
+
if(originalParent instanceof DoubleOperatorClause){
-
+
rightChild = rightChild((DoubleOperatorClause)originalParent) ==
originalChild
? newChild
: rightChild((DoubleOperatorClause)originalParent);
@@ -218,10 +229,10 @@
/** Create a new operator clause, of type opCls, with the left and right
children.
* We must also specify for whom it is to be a replacement for.
* The replacementFor must be from the original branch.
- ** @param left
- * @param right
- * @param replacementFor
- * @return
+ ** @param left
+ * @param right
+ * @param replacementFor
+ * @return
*/
protected <T extends OperationClause> T createOperatorClause(
final Clause left,
@@ -245,10 +256,10 @@
}else if (NotClause.class.isAssignableFrom(replacementFor.getClass())){
clause = (T) context.createNotClause(left);
-
+
}else if
(AndNotClause.class.isAssignableFrom(replacementFor.getClass())){
clause = (T) context.createAndNotClause(left);
-
+
}
return clause;
@@ -256,7 +267,7 @@
/** Create XorClauses required to present all the alternatives in the
query tree.
* There will be alternatives.size()-1 XorClauses aligned in a
right-leaning branch.
- *
+ *
* @param alternatives what will be leaves of the right-leaning XorClause
branch returned
* @return the right-leaning XorClause branch
*/
@@ -269,13 +280,13 @@
}
/** What XorClause.Hint is used for newly created XorClause alternations.
- *
+ *
* @return the XorClause.Hint used during this alternation process.
*/
protected abstract XorClause.Hint getAlternationHint();
-
+
// Private -------------------------------------------------------
-
+
// Inner classes -------------------------------------------------
-
+
}
_______________________________________________
Kernel-commits mailing list
[email protected]
http://sesat.no/mailman/listinfo/kernel-commits
