# HG changeset patch # User Severin Gehwolf # Date 1373619580 -7200 # Node ID 93781d53f6be20cf57bd75e214119154268c9398 # Parent 23fd98e86b026928f30bd8bfe86016e57f6ef14e Fix parsing/tree building of multiple conjunctions/disjunctions. Reviewed-by: ebaron Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2013-July/007315.html diff -r 23fd98e86b02 -r 93781d53f6be storage/core/src/main/java/com/redhat/thermostat/storage/internal/statement/Node.java --- a/storage/core/src/main/java/com/redhat/thermostat/storage/internal/statement/Node.java Fri Jul 12 14:25:15 2013 -0400 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/internal/statement/Node.java Fri Jul 12 10:59:40 2013 +0200 @@ -77,7 +77,7 @@ System.out.println(""); if (value instanceof Node) { Node node = (Node)value; - node.print(level++); + node.print(level + 1); } } diff -r 23fd98e86b02 -r 93781d53f6be storage/core/src/main/java/com/redhat/thermostat/storage/internal/statement/StatementDescriptorParser.java --- a/storage/core/src/main/java/com/redhat/thermostat/storage/internal/statement/StatementDescriptorParser.java Fri Jul 12 14:25:15 2013 -0400 +++ b/storage/core/src/main/java/com/redhat/thermostat/storage/internal/statement/StatementDescriptorParser.java Fri Jul 12 10:59:40 2013 +0200 @@ -66,7 +66,7 @@ * 'LIMIT' term | \empty * where := whereExp sort limit * whereExp := andCond orCond - * orCond := 'OR' andCond | \empty + * orCond := 'OR' whereExp | \empty * sort := 'SORT' sortCond | \empty * sortCond := sortPair sortList * sortPair := literal sortModifier @@ -74,7 +74,7 @@ * sortList := ',' sortCond | \empty * limit := 'LIMIT' term | \empty * andCond := condition andBody - * andBody := 'AND' condition | \empty + * andBody := 'AND' whereExp | \empty * condition := 'NOT' condition | compExp * compExp := term compExpRHS * term := freeParam | literal @@ -165,7 +165,7 @@ currTokenIndex++; WhereExpression expn = new WhereExpression(); tree.setWhereExpn(expn); - matchWhereExp(expn); + matchWhereExp(expn.getRoot()); matchSort(tree); matchLimit(tree); } else if (tokens[currTokenIndex].equals(KEYWORD_SORT)) { @@ -248,12 +248,13 @@ } } - private void matchWhereExp(WhereExpression expn) throws DescriptorParsingException { + private void matchWhereExp(Node node) throws DescriptorParsingException { if (currTokenIndex >= tokens.length) { throw new DescriptorParsingException("Illegal where clause"); } - matchAndCondition(expn.getRoot()); - matchOrCondition(expn.getRoot()); + assert(node != null); + matchAndCondition(node); + matchOrCondition(node); } private void matchAndCondition(Node currNode) throws DescriptorParsingException { @@ -296,13 +297,15 @@ TerminalNode right = new TerminalNode(expr); expr.setLeftChild(left); expr.setRightChild(right); + if (currNode instanceof BinaryExpressionNode) { BinaryExpressionNode currNodeExpr = (BinaryExpressionNode)currNode; Node available = currNodeExpr.getLeftChild(); - if (available != null) { + if (available == null) { + currNodeExpr.setLeftChild(expr); + } else { + assert(currNodeExpr.getRightChild() == null); currNodeExpr.setRightChild(expr); - } else { - currNodeExpr.setLeftChild(expr); } } else { assert(currNode instanceof NotBooleanExpressionNode || currNode instanceof Node); @@ -497,14 +500,31 @@ if (currTokenIndex < tokens.length && tokens[currTokenIndex].equals(Operator.AND.getName())) { currTokenIndex++; // AND keyword - Node parent = currNode.getParent(); + + Node parent = currNode; + if (currNode instanceof BinaryExpressionNode || + currNode instanceof NotBooleanExpressionNode) { + parent = currNode.getParent(); + assert(parent != null); + } BinaryExpressionNode and = new BinaryExpressionNode(parent); - and.setLeftChild((Node)currNode.getValue()); - currNode.setParent(and); and.setOperator(BinaryLogicalOperator.AND); - currNode.setValue(and); + if (currNode instanceof BinaryExpressionNode || + currNode instanceof NotBooleanExpressionNode) { + currNode.setParent(and); + and.setLeftChild(currNode); + parent.setValue(and); + } else { + // Root node case + and.setLeftChild((Node)parent.getValue()); + parent.setValue(and); + } + // Note the current AND expression node at this point of parsing + // must be at the root of the entire expression. + assert(and.getParent().getParent() == null); - matchCondition(and); + matchWhereExp(and); + } // empty } @@ -513,14 +533,30 @@ if (currTokenIndex < tokens.length && tokens[currTokenIndex].equals(Operator.OR.getName())) { currTokenIndex++; // OR keyword - Node parent = currNode.getParent(); + + Node parent = currNode; + if (currNode instanceof BinaryExpressionNode || + currNode instanceof NotBooleanExpressionNode) { + parent = currNode.getParent(); + assert(parent != null); + } BinaryExpressionNode or = new BinaryExpressionNode(parent); - or.setLeftChild((Node)currNode.getValue()); - currNode.setParent(or); or.setOperator(BinaryLogicalOperator.OR); - currNode.setValue(or); + if (currNode instanceof BinaryExpressionNode || + currNode instanceof NotBooleanExpressionNode) { + currNode.setParent(or); + or.setLeftChild(currNode); + parent.setValue(or); + } else { + // Root node case + or.setLeftChild((Node)parent.getValue()); + parent.setValue(or); + } + // Note the current OR expression node at this point of parsing + // must be at the root of the entire expression. + assert(or.getParent().getParent() == null); - matchAndCondition(or); + matchWhereExp(or); } // empty } diff -r 23fd98e86b02 -r 93781d53f6be storage/core/src/test/java/com/redhat/thermostat/storage/internal/statement/StatementDescriptorParserTest.java --- a/storage/core/src/test/java/com/redhat/thermostat/storage/internal/statement/StatementDescriptorParserTest.java Fri Jul 12 14:25:15 2013 -0400 +++ b/storage/core/src/test/java/com/redhat/thermostat/storage/internal/statement/StatementDescriptorParserTest.java Fri Jul 12 10:59:40 2013 +0200 @@ -154,6 +154,374 @@ @SuppressWarnings("rawtypes") @Test + public void testParseQueryWithMultipleConcunctions() throws DescriptorParsingException { + String descrString = "QUERY " + AgentInfoDAO.CATEGORY.getName() + " WHERE 'a' = 'b' AND 'c' = 'd' AND 'e' < ?i"; + StatementDescriptor desc = getDescriptor(descrString, AgentInfoDAO.CATEGORY); + parser = new StatementDescriptorParser(storage, desc); + ParsedStatement statement = parser.parse(); + assertEquals(1, statement.getNumParams()); + assertEquals(mockQuery.getClass().getName(), statement.getRawStatement().getClass().getName()); + SuffixExpression expn = statement.getSuffixExpression(); + assertNull(expn.getLimitExpn()); + assertNull(expn.getSortExpn()); + assertNotNull(expn.getWhereExpn()); + WhereExpression where = expn.getWhereExpn(); + // build the expected expression tree + WhereExpression expected = new WhereExpression(); + BinaryExpressionNode and1 = new BinaryExpressionNode(expected.getRoot()); + expected.getRoot().setValue(and1); + BinaryExpressionNode and2 = new BinaryExpressionNode(and1); + and1.setLeftChild(and2); + and1.setOperator(BinaryLogicalOperator.AND); + BinaryExpressionNode equality1 = new BinaryExpressionNode(and2); + and2.setOperator(BinaryLogicalOperator.AND); + and2.setLeftChild(equality1); + equality1.setOperator(BinaryComparisonOperator.EQUALS); + TerminalNode a = new TerminalNode(equality1); + Key aKey = new Key("a", false); + a.setValue(aKey); + equality1.setLeftChild(a); + TerminalNode b = new TerminalNode(equality1); + b.setValue("b"); + equality1.setRightChild(b); + BinaryExpressionNode equality2 = new BinaryExpressionNode(and2); + and2.setRightChild(equality2); + equality2.setOperator(BinaryComparisonOperator.EQUALS); + TerminalNode c = new TerminalNode(equality2); + Key cKey = new Key("c", false); + c.setValue(cKey); + equality2.setLeftChild(c); + TerminalNode d = new TerminalNode(equality2); + d.setValue("d"); + equality2.setRightChild(d); + BinaryExpressionNode lessThan = new BinaryExpressionNode(and1); + lessThan.setOperator(BinaryComparisonOperator.LESS_THAN); + and1.setRightChild(lessThan); + TerminalNode e = new TerminalNode(lessThan); + Key eKey = new Key("e", false); + e.setValue(eKey); + lessThan.setLeftChild(e); + UnfinishedValueNode f = new UnfinishedValueNode(); + f.setParameterIndex(0); + f.setType(Integer.class); + f.setLHS(false); + TerminalNode fReal = new TerminalNode(lessThan); + fReal.setValue(f); + lessThan.setRightChild(fReal); + + assertTrue( WhereExpressions.equals(expected, where)); + } + + @SuppressWarnings("rawtypes") + @Test + public void testParseQueryWithMultipleConcunctions2() throws DescriptorParsingException { + String descrString = "QUERY " + AgentInfoDAO.CATEGORY.getName() + " WHERE 'a' = 'b' AND 'c' = 'd' AND 'e' < 'f' AND 'g' >= 'h'"; + StatementDescriptor desc = getDescriptor(descrString, AgentInfoDAO.CATEGORY); + parser = new StatementDescriptorParser(storage, desc); + ParsedStatement statement = parser.parse(); + assertEquals(0, statement.getNumParams()); + assertEquals(mockQuery.getClass().getName(), statement.getRawStatement().getClass().getName()); + SuffixExpression expn = statement.getSuffixExpression(); + assertNull(expn.getLimitExpn()); + assertNull(expn.getSortExpn()); + assertNotNull(expn.getWhereExpn()); + WhereExpression where = expn.getWhereExpn(); + // build the expected expression tree + WhereExpression expected = new WhereExpression(); + BinaryExpressionNode and1 = new BinaryExpressionNode(expected.getRoot()); + expected.getRoot().setValue(and1); + BinaryExpressionNode and2 = new BinaryExpressionNode(and1); + and1.setLeftChild(and2); + BinaryExpressionNode and3 = new BinaryExpressionNode(and2); + and3.setOperator(BinaryLogicalOperator.AND); + and2.setLeftChild(and3); + and1.setOperator(BinaryLogicalOperator.AND); + BinaryExpressionNode equality1 = new BinaryExpressionNode(and3); + and2.setOperator(BinaryLogicalOperator.AND); + and3.setLeftChild(equality1); + equality1.setOperator(BinaryComparisonOperator.EQUALS); + TerminalNode a = new TerminalNode(equality1); + Key aKey = new Key("a", false); + a.setValue(aKey); + equality1.setLeftChild(a); + TerminalNode b = new TerminalNode(equality1); + b.setValue("b"); + equality1.setRightChild(b); + BinaryExpressionNode equality2 = new BinaryExpressionNode(and3); + and3.setRightChild(equality2); + equality2.setOperator(BinaryComparisonOperator.EQUALS); + TerminalNode c = new TerminalNode(equality2); + Key cKey = new Key("c", false); + c.setValue(cKey); + equality2.setLeftChild(c); + TerminalNode d = new TerminalNode(equality2); + d.setValue("d"); + equality2.setRightChild(d); + BinaryExpressionNode lessThan = new BinaryExpressionNode(and2); + lessThan.setOperator(BinaryComparisonOperator.LESS_THAN); + and2.setRightChild(lessThan); + TerminalNode e = new TerminalNode(lessThan); + Key eKey = new Key("e", false); + e.setValue(eKey); + lessThan.setLeftChild(e); + TerminalNode f = new TerminalNode(lessThan); + f.setValue("f"); + lessThan.setRightChild(f); + BinaryExpressionNode greaterOrEqual = new BinaryExpressionNode(and1); + greaterOrEqual.setOperator(BinaryComparisonOperator.GREATER_THAN_OR_EQUAL_TO); + TerminalNode g = new TerminalNode(greaterOrEqual); + Key gKey = new Key("g", false); + g.setValue(gKey); + greaterOrEqual.setLeftChild(g); + TerminalNode h = new TerminalNode(greaterOrEqual); + h.setValue("h"); + greaterOrEqual.setRightChild(h); + and1.setRightChild(greaterOrEqual); + + assertTrue( WhereExpressions.equals(expected, where)); + } + + @SuppressWarnings("rawtypes") + @Test + public void testParseQueryWithMultipleDisjunctions() throws DescriptorParsingException { + String descrString = "QUERY " + AgentInfoDAO.CATEGORY.getName() + " WHERE 'a' = 'b' OR 'c' = 'd' OR 'e' < ?i"; + StatementDescriptor desc = getDescriptor(descrString, AgentInfoDAO.CATEGORY); + parser = new StatementDescriptorParser(storage, desc); + ParsedStatement statement = parser.parse(); + assertEquals(1, statement.getNumParams()); + assertEquals(mockQuery.getClass().getName(), statement.getRawStatement().getClass().getName()); + SuffixExpression expn = statement.getSuffixExpression(); + assertNull(expn.getLimitExpn()); + assertNull(expn.getSortExpn()); + assertNotNull(expn.getWhereExpn()); + + WhereExpression where = expn.getWhereExpn(); + // build the expected expression tree + WhereExpression expected = new WhereExpression(); + BinaryExpressionNode or1 = new BinaryExpressionNode(expected.getRoot()); + expected.getRoot().setValue(or1); + BinaryExpressionNode or2 = new BinaryExpressionNode(or1); + or1.setLeftChild(or2); + or1.setOperator(BinaryLogicalOperator.OR); + BinaryExpressionNode equality1 = new BinaryExpressionNode(or2); + or2.setOperator(BinaryLogicalOperator.OR); + or2.setLeftChild(equality1); + equality1.setOperator(BinaryComparisonOperator.EQUALS); + TerminalNode a = new TerminalNode(equality1); + Key aKey = new Key("a", false); + a.setValue(aKey); + equality1.setLeftChild(a); + TerminalNode b = new TerminalNode(equality1); + b.setValue("b"); + equality1.setRightChild(b); + BinaryExpressionNode equality2 = new BinaryExpressionNode(or2); + or2.setRightChild(equality2); + equality2.setOperator(BinaryComparisonOperator.EQUALS); + TerminalNode c = new TerminalNode(equality2); + Key cKey = new Key("c", false); + c.setValue(cKey); + equality2.setLeftChild(c); + TerminalNode d = new TerminalNode(equality2); + d.setValue("d"); + equality2.setRightChild(d); + BinaryExpressionNode lessThan = new BinaryExpressionNode(or1); + lessThan.setOperator(BinaryComparisonOperator.LESS_THAN); + or1.setRightChild(lessThan); + TerminalNode e = new TerminalNode(lessThan); + Key eKey = new Key("e", false); + e.setValue(eKey); + lessThan.setLeftChild(e); + UnfinishedValueNode f = new UnfinishedValueNode(); + f.setParameterIndex(0); + f.setType(Integer.class); + f.setLHS(false); + TerminalNode fReal = new TerminalNode(lessThan); + fReal.setValue(f); + lessThan.setRightChild(fReal); + + assertTrue( WhereExpressions.equals(expected, where)); + } + + @SuppressWarnings("rawtypes") + @Test + public void testParseQueryWithMultipleDisjunctions2() throws DescriptorParsingException { + String descrString = "QUERY " + AgentInfoDAO.CATEGORY.getName() + " WHERE 'a' = 'b' OR 'c' = 'd' OR 'e' < 'f' OR 'g' >= 'h'"; + StatementDescriptor desc = getDescriptor(descrString, AgentInfoDAO.CATEGORY); + parser = new StatementDescriptorParser(storage, desc); + ParsedStatement statement = parser.parse(); + assertEquals(0, statement.getNumParams()); + assertEquals(mockQuery.getClass().getName(), statement.getRawStatement().getClass().getName()); + SuffixExpression expn = statement.getSuffixExpression(); + assertNull(expn.getLimitExpn()); + assertNull(expn.getSortExpn()); + assertNotNull(expn.getWhereExpn()); + + WhereExpression where = expn.getWhereExpn(); + // build the expected expression tree + WhereExpression expected = new WhereExpression(); + BinaryExpressionNode or1 = new BinaryExpressionNode(expected.getRoot()); + expected.getRoot().setValue(or1); + BinaryExpressionNode or2 = new BinaryExpressionNode(or1); + or1.setLeftChild(or2); + BinaryExpressionNode or3 = new BinaryExpressionNode(or2); + or3.setOperator(BinaryLogicalOperator.OR); + or2.setLeftChild(or3); + or1.setOperator(BinaryLogicalOperator.OR); + BinaryExpressionNode equality1 = new BinaryExpressionNode(or3); + or2.setOperator(BinaryLogicalOperator.OR); + or3.setLeftChild(equality1); + equality1.setOperator(BinaryComparisonOperator.EQUALS); + TerminalNode a = new TerminalNode(equality1); + Key aKey = new Key("a", false); + a.setValue(aKey); + equality1.setLeftChild(a); + TerminalNode b = new TerminalNode(equality1); + b.setValue("b"); + equality1.setRightChild(b); + BinaryExpressionNode equality2 = new BinaryExpressionNode(or3); + or3.setRightChild(equality2); + equality2.setOperator(BinaryComparisonOperator.EQUALS); + TerminalNode c = new TerminalNode(equality2); + Key cKey = new Key("c", false); + c.setValue(cKey); + equality2.setLeftChild(c); + TerminalNode d = new TerminalNode(equality2); + d.setValue("d"); + equality2.setRightChild(d); + BinaryExpressionNode lessThan = new BinaryExpressionNode(or2); + lessThan.setOperator(BinaryComparisonOperator.LESS_THAN); + or2.setRightChild(lessThan); + TerminalNode e = new TerminalNode(lessThan); + Key eKey = new Key("e", false); + e.setValue(eKey); + lessThan.setLeftChild(e); + TerminalNode f = new TerminalNode(lessThan); + f.setValue("f"); + lessThan.setRightChild(f); + BinaryExpressionNode greaterOrEqual = new BinaryExpressionNode(or1); + greaterOrEqual.setOperator(BinaryComparisonOperator.GREATER_THAN_OR_EQUAL_TO); + TerminalNode g = new TerminalNode(greaterOrEqual); + Key gKey = new Key("g", false); + g.setValue(gKey); + greaterOrEqual.setLeftChild(g); + TerminalNode h = new TerminalNode(greaterOrEqual); + h.setValue("h"); + greaterOrEqual.setRightChild(h); + or1.setRightChild(greaterOrEqual); + + assertTrue( WhereExpressions.equals(expected, where)); + } + + @SuppressWarnings("rawtypes") + @Test + public void testParseQueryWithMultipleConDisjunctions() throws DescriptorParsingException { + String descrString = "QUERY " + AgentInfoDAO.CATEGORY.getName() + " WHERE 'a' = 'b' OR 'c' = 'd' OR 'e' < 'f' OR 'g' >= 'h' AND 'x' = 'y' AND 'u' = 'w' AND 's' = 't'"; + StatementDescriptor desc = getDescriptor(descrString, AgentInfoDAO.CATEGORY); + parser = new StatementDescriptorParser(storage, desc); + ParsedStatement statement = parser.parse(); + assertEquals(0, statement.getNumParams()); + assertEquals(mockQuery.getClass().getName(), statement.getRawStatement().getClass().getName()); + SuffixExpression expn = statement.getSuffixExpression(); + assertNull(expn.getLimitExpn()); + assertNull(expn.getSortExpn()); + assertNotNull(expn.getWhereExpn()); + + WhereExpression where = expn.getWhereExpn(); + // build the expected expression tree + WhereExpression expected = new WhereExpression(); + BinaryExpressionNode and3 = new BinaryExpressionNode(expected.getRoot()); + expected.getRoot().setValue(and3); + and3.setOperator(BinaryLogicalOperator.AND); + BinaryExpressionNode and2 = new BinaryExpressionNode(and3); + and2.setOperator(BinaryLogicalOperator.AND); + BinaryExpressionNode and1 = new BinaryExpressionNode(and2); + and1.setOperator(BinaryLogicalOperator.AND); + BinaryExpressionNode equality3 = new BinaryExpressionNode(and1); + equality3.setOperator(BinaryComparisonOperator.EQUALS); + TerminalNode x = new TerminalNode(equality3); + x.setValue(new Key("x", false)); + TerminalNode y = new TerminalNode(equality3); + y.setValue("y"); + equality3.setLeftChild(x); + equality3.setRightChild(y); + and1.setRightChild(equality3); + and3.setLeftChild(and2); + and2.setLeftChild(and1); + BinaryExpressionNode equality4 = new BinaryExpressionNode(and2); + equality4.setOperator(BinaryComparisonOperator.EQUALS); + and2.setRightChild(equality4); + TerminalNode u = new TerminalNode(equality4); + u.setValue(new Key("u", false)); + equality4.setLeftChild(u); + TerminalNode w = new TerminalNode(equality4); + w.setValue("w"); + equality4.setRightChild(w); + BinaryExpressionNode equality5 = new BinaryExpressionNode(and3); + equality5.setOperator(BinaryComparisonOperator.EQUALS); + TerminalNode s = new TerminalNode(equality5); + s.setValue(new Key("s", false)); + TerminalNode t = new TerminalNode(equality5); + t.setValue("t"); + equality5.setLeftChild(s); + equality5.setRightChild(t); + and3.setRightChild(equality5); + BinaryExpressionNode or3 = new BinaryExpressionNode(and1); + BinaryExpressionNode or2 = new BinaryExpressionNode(or3); + BinaryExpressionNode or1 = new BinaryExpressionNode(or2); + or3.setOperator(BinaryLogicalOperator.OR); + or2.setOperator(BinaryLogicalOperator.OR); + or1.setOperator(BinaryLogicalOperator.OR); + or3.setLeftChild(or2); + or2.setLeftChild(or1); + BinaryExpressionNode equality1 = new BinaryExpressionNode(or1); + or1.setLeftChild(equality1); + equality1.setOperator(BinaryComparisonOperator.EQUALS); + TerminalNode a = new TerminalNode(equality1); + Key aKey = new Key("a", false); + a.setValue(aKey); + equality1.setLeftChild(a); + TerminalNode b = new TerminalNode(equality1); + b.setValue("b"); + equality1.setRightChild(b); + BinaryExpressionNode equality2 = new BinaryExpressionNode(or1); + equality2.setOperator(BinaryComparisonOperator.EQUALS); + or1.setRightChild(equality2); + TerminalNode c = new TerminalNode(equality2); + Key cKey = new Key("c", false); + c.setValue(cKey); + equality2.setLeftChild(c); + TerminalNode d = new TerminalNode(equality2); + d.setValue("d"); + equality2.setRightChild(d); + BinaryExpressionNode lessThan = new BinaryExpressionNode(or2); + lessThan.setOperator(BinaryComparisonOperator.LESS_THAN); + or2.setRightChild(lessThan); + or2.setLeftChild(or1); + TerminalNode e = new TerminalNode(lessThan); + Key eKey = new Key("e", false); + e.setValue(eKey); + lessThan.setLeftChild(e); + TerminalNode f = new TerminalNode(lessThan); + f.setValue("f"); + lessThan.setRightChild(f); + BinaryExpressionNode greaterOrEqual = new BinaryExpressionNode(or3); + greaterOrEqual.setOperator(BinaryComparisonOperator.GREATER_THAN_OR_EQUAL_TO); + TerminalNode g = new TerminalNode(greaterOrEqual); + Key gKey = new Key("g", false); + g.setValue(gKey); + greaterOrEqual.setLeftChild(g); + TerminalNode h = new TerminalNode(greaterOrEqual); + h.setValue("h"); + greaterOrEqual.setRightChild(h); + or3.setRightChild(greaterOrEqual); + or3.setLeftChild(or2); + and1.setLeftChild(or3); + + assertTrue(WhereExpressions.equals(expected, where)); + } + + @SuppressWarnings("rawtypes") + @Test public void testParseQueryWhereAndSortMultiple() throws DescriptorParsingException { String descrString = "QUERY " + AgentInfoDAO.CATEGORY.getName() + " WHERE 'a' < 'b' AND 'c' = ?s OR NOT 'x' >= ?i SORT 'a' ASC , 'b' DSC , 'c' ASC"; StatementDescriptor desc = getDescriptor(descrString, AgentInfoDAO.CATEGORY);