Skip to content

Commit

Permalink
fixes #1576
Browse files Browse the repository at this point in the history
  • Loading branch information
wumpz committed Jul 9, 2022
1 parent d5f349d commit 48ea0e2
Show file tree
Hide file tree
Showing 4 changed files with 170 additions and 74 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,6 @@ public class CNFConverter {
// notice temp1 will be settled as the root and temp2 will be
// settled as the dummy root.
private boolean isUsed = false;
private CloneHelper clone = new CloneHelper();

private class Mule {

Expand All @@ -234,9 +233,8 @@ public static Expression convertToCNF(Expression expr) {
}

/**
* this method takes an expression tree and converts that into a CNF form. Notice the 5 steps
* shown above will turn into 5 different methods. For the sake of testing, I set them public.
* return the converted expression.
* this method takes an expression tree and converts that into a CNF form. Notice the 5 steps shown above will turn
* into 5 different methods. For the sake of testing, I set them public. return the converted expression.
*
* @param express the original expression tree.
*/
Expand All @@ -260,21 +258,21 @@ private Expression convert(Expression express)
}

/**
* this is the first step that rebuild the expression tree. Use the standard specified in the
* above class. Traverse the original tree recursively and rebuild the tree from that.
* this is the first step that rebuild the expression tree. Use the standard specified in the above class. Traverse
* the original tree recursively and rebuild the tree from that.
*
* @param express the original expression tree.
*/
private void reorder(Expression express) {
root = clone.modify(express);
root = CloneHelper.modify(express);
List<Expression> list = new ArrayList<Expression>();
list.add(root);
dummy = new MultiAndExpression(list);
}

/**
* This method is used to deal with pushing not operators down. Since it needs an extra
* parameter, I will create a new method to handle this.
* This method is used to deal with pushing not operators down. Since it needs an extra parameter, I will create a
* new method to handle this.
*/
private void pushNotDown() {
/* set the two temp parameters to their staring point. */
Expand All @@ -290,11 +288,10 @@ private void pushNotDown() {
}

/**
* This method is the helper function to push not operators down. traverse the tree thoroughly,
* when we meet the not operator. We only need to consider these three operators:
* MultiAndOperator, MultiOrOperator, NotOperator. Handle them in a seperate way. when we finish
* the traverse, the expression tree will have all the not operators pushed as downwards as they
* could. In the method, I use two global variables: temp1 and temp2 to traverse the expression
* This method is the helper function to push not operators down. traverse the tree thoroughly, when we meet the not
* operator. We only need to consider these three operators: MultiAndOperator, MultiOrOperator, NotOperator. Handle
* them in a seperate way. when we finish the traverse, the expression tree will have all the not operators pushed
* as downwards as they could. In the method, I use two global variables: temp1 and temp2 to traverse the expression
* tree. Notice that temp2 will always be the parent of temp1.
*
* @param index the index of the children appeared in parents array.
Expand Down Expand Up @@ -322,8 +319,8 @@ private void pushNot(int index) {
}

/**
* This function mainly deals with pushing not operators down. check the child. If it is not a
* logic operator(and or or). stop at that point. Else use De Morgan law to push not downwards.
* This function mainly deals with pushing not operators down. check the child. If it is not a logic operator(and or
* or). stop at that point. Else use De Morgan law to push not downwards.
*
* @param index the index of the children appeared in parents array.
*/
Expand Down Expand Up @@ -386,10 +383,9 @@ private void handleNot(int index) {
}

/**
* This method serves as dealing with the third step. It is used to put all the adjacent same
* multi operators together. BFS the tree and do it node by node. In the end we will get the
* tree where all the same multi operators store in the same odd level of the tree or in the
* same even level of the tree.
* This method serves as dealing with the third step. It is used to put all the adjacent same multi operators
* together. BFS the tree and do it node by node. In the end we will get the tree where all the same multi operators
* store in the same odd level of the tree or in the same even level of the tree.
*/
@SuppressWarnings({"PMD.CyclomaticComplexity"})
private void gather() {
Expand Down Expand Up @@ -468,10 +464,10 @@ private void gather() {
}

/**
* First, BFS the tree and gather all the or operators and their parents into a stack. Next, pop
* them out and push the and operators under the or operators upwards(if there are). Do this
* level by level, which means during each level we will call the gather() method to make the
* tree uniform. When we move out of the stack. The expression tree shall be in CNF form.
* First, BFS the tree and gather all the or operators and their parents into a stack. Next, pop them out and push
* the and operators under the or operators upwards(if there are). Do this level by level, which means during each
* level we will call the gather() method to make the tree uniform. When we move out of the stack. The expression
* tree shall be in CNF form.
*/
private void pushAndUp() {
Queue<Mule> queue = new LinkedList<Mule>();
Expand Down Expand Up @@ -517,12 +513,11 @@ private void pushAndUp() {
}

/**
* This helper function is used to deal with pushing and up: generally, pop the top element out
* of the stack, use BFS to traverse the tree and push and up. It will case the expression tree
* to have the and as the new root and multiple or as the children. Push them on the queue and
* repeat the same process until the newly generated or operator does not have any and operators
* in it(which means no elements will be added into the queue). when one level is finished,
* regroup the tree. Do this until the stack is empty, the result will be the expression in CNF
* This helper function is used to deal with pushing and up: generally, pop the top element out of the stack, use
* BFS to traverse the tree and push and up. It will case the expression tree to have the and as the new root and
* multiple or as the children. Push them on the queue and repeat the same process until the newly generated or
* operator does not have any and operators in it(which means no elements will be added into the queue). when one
* level is finished, regroup the tree. Do this until the stack is empty, the result will be the expression in CNF
* form.
*
* @param stack the stack stores a list of combined data.
Expand Down Expand Up @@ -570,7 +565,7 @@ private void pushAnd(Stack<Mule> stack) {
MultiAndExpression newand = new MultiAndExpression(list);
parents.setChild(parents.getIndex(children), newand);
for (int i = 0; i < and.size(); i++) {
Expression temp = clone.shallowCopy(children);
Expression temp = CloneHelper.shallowCopy(children);
MultipleExpression mtemp = (MultipleExpression) temp;
mtemp.addChild(mtemp.size(), and.getChild(i));
newand.addChild(i, mtemp);
Expand All @@ -581,21 +576,20 @@ private void pushAnd(Stack<Mule> stack) {
}

/**
* This is the final step of the CNF conversion: now we have the Expression tree that has one
* multiple and expression with a list of multiple or expression as the child. So we need to
* convert the multiple expression back to the binary counterparts. Note the converted tree is
* left inclined. Also I attach a parenthesis node before the or expression that is attached to
* the and expression to make the generated result resembles the CNF form.
* This is the final step of the CNF conversion: now we have the Expression tree that has one multiple and
* expression with a list of multiple or expression as the child. So we need to convert the multiple expression back
* to the binary counterparts. Note the converted tree is left inclined. Also I attach a parenthesis node before the
* or expression that is attached to the and expression to make the generated result resembles the CNF form.
*/
private void changeBack() {
if (!(root instanceof MultiAndExpression)) {
return;
}
MultipleExpression temp = (MultipleExpression) root;
for (int i = 0; i < temp.size(); i++) {
temp.setChild(i, clone.changeBack(true, temp.getChild(i)));
temp.setChild(i, CloneHelper.changeBack(true, temp.getChild(i)));
}
root = clone.changeBack(false, temp);
root = CloneHelper.changeBack(false, temp);
}

}
72 changes: 46 additions & 26 deletions src/main/java/net/sf/jsqlparser/util/cnfexpression/CloneHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,16 @@
import net.sf.jsqlparser.expression.operators.conditional.OrExpression;

/**
* This class is mainly used for handling the cloning of an expression tree.
* Note this is the shallow copy of the tree. That means I do not modify
* or copy the expression other than these expressions:
* AND, OR, NOT, (), MULTI-AND, MULTI-OR.
* Since the CNF conversion only change the condition part of the tree.
* This class is mainly used for handling the cloning of an expression tree. Note this is the shallow copy of the tree.
* That means I do not modify or copy the expression other than these expressions: AND, OR, NOT, (), MULTI-AND,
* MULTI-OR. Since the CNF conversion only change the condition part of the tree.
*
* @author messfish
*
*/
class CloneHelper {

public Expression modify(Expression express) {
public static Expression modify(Expression express) {
if (express instanceof NotExpression) {
return new NotExpression(modify(((NotExpression) express).getExpression()));
}
Expand All @@ -40,7 +38,7 @@ public Expression modify(Expression express) {
}
if (express instanceof AndExpression) {
AndExpression and = (AndExpression) express;
List<Expression> list = new ArrayList<Expression>();
List<Expression> list = new ArrayList<>();
list.add(modify(and.getLeftExpression()));
list.add(modify(and.getRightExpression()));
MultiAndExpression result = new MultiAndExpression(list);
Expand All @@ -51,7 +49,7 @@ public Expression modify(Expression express) {
}
if (express instanceof OrExpression) {
OrExpression or = (OrExpression) express;
List<Expression> list = new ArrayList<Expression>();
List<Expression> list = new ArrayList<>();
list.add(modify(or.getLeftExpression()));
list.add(modify(or.getRightExpression()));
MultiOrExpression result = new MultiOrExpression(list);
Expand All @@ -71,16 +69,16 @@ public Expression modify(Expression express) {
}

/**
* This method is used to copy the expression which happens at step four. I only copy the
* conditional expressions since the CNF only changes the conditional part.
* This method is used to copy the expression which happens at step four. I only copy the conditional expressions
* since the CNF only changes the conditional part.
*
* @param express the expression that will be copied.
* @return the copied expression.
*/
public Expression shallowCopy(Expression express) {
public static Expression shallowCopy(Expression express) {
if (express instanceof MultipleExpression) {
MultipleExpression multi = (MultipleExpression) express;
List<Expression> list = new ArrayList<Expression>();
List<Expression> list = new ArrayList<>();
for (int i = 0; i < multi.size(); i++) {
list.add(shallowCopy(multi.getChild(i)));
}
Expand All @@ -95,32 +93,54 @@ public Expression shallowCopy(Expression express) {
}

/**
* This helper method is used to change the multiple expression into the binary form,
* respectively and return the root of the expression tree.
* This helper method is used to change the multiple expression into the binary form, respectively and return the
* root of the expression tree.
*
* @param isMultiOr variable tells whether the expression is or.
* @param exp the expression that needs to be converted.
* @return the root of the expression tree.
*/
public Expression changeBack(Boolean isMultiOr, Expression exp) {
public static Expression changeBack(Boolean isMultiOr, Expression exp) {
if (!(exp instanceof MultipleExpression)) {
return exp;
}
MultipleExpression changed = (MultipleExpression) exp;
Expression result = changed.getChild(0);
for (int i = 1; i < changed.size(); i++) {
Expression left = result;
Expression right = changed.getChild(i);
if (isMultiOr) {
result = new OrExpression(left, right);
} else {
result = new AndExpression(left, right);

List<Expression> result = ((MultipleExpression) exp).getList();
while (result.size() > 1) {
List<Expression> compressed = new ArrayList<>();
for (int i = 0; i < result.size(); i = i + 2) {
Expression left = result.get(i);
Expression right = i + 1 < result.size() ? result.get(i + 1) : null;

if (isMultiOr) {
compressed.add(right != null ? new OrExpression(left, right) : left);
} else {
compressed.add(right != null ? new AndExpression(left, right) : left);
}
}
result = compressed;
}
if (isMultiOr) {
return new Parenthesis(result);
return new Parenthesis(result.get(0));
} else {
return result.get(0);
}
return result;

// MultipleExpression changed = (MultipleExpression) exp;
// Expression result = changed.getChild(0);
// for (int i = 1; i < changed.size(); i++) {
// Expression left = result;
// Expression right = changed.getChild(i);
// if (isMultiOr) {
// result = new OrExpression(left, right);
// } else {
// result = new AndExpression(left, right);
// }
// }
// if (isMultiOr) {
// return new Parenthesis(result);
// }
// return result;
}

}
Loading

0 comments on commit 48ea0e2

Please sign in to comment.