db-derby-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kahat...@apache.org
Subject svn commit: r887156 - in /db/derby/code/trunk/java: engine/org/apache/derby/impl/sql/compile/ testing/org/apache/derbyTesting/functionTests/tests/lang/
Date Fri, 04 Dec 2009 11:02:17 GMT
Author: kahatlen
Date: Fri Dec  4 11:02:16 2009
New Revision: 887156

URL: http://svn.apache.org/viewvc?rev=887156&view=rev
Log:
DERBY-2282: Incorrect "transitive closure" logic leads to inconsistent behavior for binary
comparison predicates

Made the transitive closure logic normalize predicates so that the
constants are on the right-hand side (e.g., convert 3 < X to X > 3).
This makes it detect more of the potential optimizations.

Added:
    db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/PredicateTest.java
  (with props)
Modified:
    db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/BinaryComparisonOperatorNode.java
    db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/BinaryOperatorNode.java
    db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/BinaryRelationalOperatorNode.java
    db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/PredicateList.java
    db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/_Suite.java

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/BinaryComparisonOperatorNode.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/BinaryComparisonOperatorNode.java?rev=887156&r1=887155&r2=887156&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/BinaryComparisonOperatorNode.java
(original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/BinaryComparisonOperatorNode.java
Fri Dec  4 11:02:16 2009
@@ -336,6 +336,25 @@
 										  ValueNode rightOperand)
 				throws StandardException;
 
+    /**
+     * <p>
+     * Return a node equivalent to this node, but with the left and right
+     * operands swapped. The node type may also be changed if the operator
+     * is not symmetric.
+     * </p>
+     *
+     * <p>
+     * This method may for instance be used to normalize a predicate by
+     * moving constants to the right-hand side of the comparison. Example:
+     * {@code 1 = A} will be transformed to {@code A = 1}, and {@code 10 < B}
+     * will be transformed to {@code B > 10}.
+     * </p>
+     *
+     * @return an equivalent expression with the operands swapped
+     * @throws StandardException if an error occurs
+     */
+    abstract BinaryOperatorNode getSwappedEquivalent() throws StandardException;
+
 	/**
 	 * Finish putting an expression into conjunctive normal
 	 * form.  An expression tree in conjunctive normal form meets

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/BinaryOperatorNode.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/BinaryOperatorNode.java?rev=887156&r1=887155&r2=887156&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/BinaryOperatorNode.java
(original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/BinaryOperatorNode.java
Fri Dec  4 11:02:16 2009
@@ -839,20 +839,6 @@
 	}
 
 	/**
-	 * Swap the left and right sides.
-	 */
-	void swapOperands()
-	{
-		String	  tmpInterfaceType = leftInterfaceType;
-		ValueNode tmpVN = leftOperand;
-
-		leftOperand = rightOperand;
-		rightOperand = tmpVN;
-		leftInterfaceType = rightInterfaceType;
-		rightInterfaceType = tmpInterfaceType;
-	}
-
-	/**
 	 * Accept the visitor for all visitable children of this node.
 	 * 
 	 * @param v the visitor

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/BinaryRelationalOperatorNode.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/BinaryRelationalOperatorNode.java?rev=887156&r1=887155&r2=887156&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/BinaryRelationalOperatorNode.java
(original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/BinaryRelationalOperatorNode.java
Fri Dec  4 11:02:16 2009
@@ -1083,7 +1083,54 @@
 
 		return -1;
 	}	
-	
+
+    /**
+     * Return an equivalent node with the operands swapped, and possibly with
+     * the operator type changed in order to preserve the meaning of the
+     * expression.
+     */
+    BinaryOperatorNode getSwappedEquivalent() throws StandardException {
+        BinaryOperatorNode newNode = (BinaryOperatorNode) getNodeFactory().getNode(getNodeTypeForSwap(),
+                rightOperand, leftOperand,
+                getContextManager());
+        newNode.setType(getTypeServices());
+        return newNode;
+    }
+
+    /**
+     * Return the node type that must be used in order to construct an
+     * equivalent expression if the operands are swapped. For symmetric
+     * operators ({@code =} and {@code <>}), the same node type is returned.
+     * Otherwise, the direction of the operator is switched in order to
+     * preserve the meaning (for instance, a node representing less-than will
+     * return the node type for greater-than).
+     *
+     * @return a node type that preserves the meaning of the expression if
+     * the operands are swapped
+     */
+    private int getNodeTypeForSwap() {
+        switch (getNodeType()) {
+            case C_NodeTypes.BINARY_EQUALS_OPERATOR_NODE:
+                return C_NodeTypes.BINARY_EQUALS_OPERATOR_NODE;
+            case C_NodeTypes.BINARY_GREATER_EQUALS_OPERATOR_NODE:
+                return C_NodeTypes.BINARY_LESS_EQUALS_OPERATOR_NODE;
+            case C_NodeTypes.BINARY_GREATER_THAN_OPERATOR_NODE:
+                return C_NodeTypes.BINARY_LESS_THAN_OPERATOR_NODE;
+            case C_NodeTypes.BINARY_LESS_THAN_OPERATOR_NODE:
+                return C_NodeTypes.BINARY_GREATER_THAN_OPERATOR_NODE;
+            case C_NodeTypes.BINARY_LESS_EQUALS_OPERATOR_NODE:
+                return C_NodeTypes.BINARY_GREATER_EQUALS_OPERATOR_NODE;
+            case C_NodeTypes.BINARY_NOT_EQUALS_OPERATOR_NODE:
+                return C_NodeTypes.BINARY_NOT_EQUALS_OPERATOR_NODE;
+            default:
+                if (SanityManager.DEBUG) {
+                    SanityManager.THROWASSERT(
+                            "Invalid nodeType: " + getNodeType());
+                }
+                return -1;
+        }
+    }
+
 	/**
 	 * is this is useful start key? for example a predicate of the from
 	 * <em>column Lessthan 5</em> is not a useful start key but is a useful stop

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/PredicateList.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/PredicateList.java?rev=887156&r1=887155&r2=887156&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/PredicateList.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/PredicateList.java Fri
Dec  4 11:02:16 2009
@@ -2271,10 +2271,10 @@
 				{
 					searchClauses.addElement(predicate);
 				}
-				else if (right instanceof ConstantNode && left instanceof ColumnReference)
+				else if (left instanceof ConstantNode && right instanceof ColumnReference)
 				{
 					// put the ColumnReference on the left to simplify things
-					bcon.swapOperands();
+					andNode.setLeftOperand(bcon.getSwappedEquivalent());
 					searchClauses.addElement(predicate);
 				}
 				continue;

Added: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/PredicateTest.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/PredicateTest.java?rev=887156&view=auto
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/PredicateTest.java
(added)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/PredicateTest.java
Fri Dec  4 11:02:16 2009
@@ -0,0 +1,137 @@
+/*
+ * Class org.apache.derbyTesting.functionTests.tests.lang.PredicateTest
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.derbyTesting.functionTests.tests.lang;
+
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.io.StringReader;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.sql.Statement;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import junit.framework.Test;
+import org.apache.derbyTesting.junit.BaseJDBCTestCase;
+import org.apache.derbyTesting.junit.JDBC;
+import org.apache.derbyTesting.junit.TestConfiguration;
+
+/**
+ * This class contains test cases for the correct handling of predicates in
+ * SQL queries.
+ */
+public class PredicateTest extends BaseJDBCTestCase {
+    public PredicateTest(String name) {
+        super(name);
+    }
+
+    public static Test suite() {
+        // We're testing engine functionality, so run in embedded only.
+        return TestConfiguration.embeddedSuite(PredicateTest.class);
+    }
+
+    /**
+     * DERBY-2282: Test that we're able to compute the transitive closure of
+     * predicates with constants on the left side of the comparison operator.
+     */
+    public void testTransitiveClosureWithConstantsOnLeftSide()
+            throws SQLException, IOException {
+
+        setAutoCommit(false); // let tables be cleaned up automatically
+
+        Statement s = createStatement();
+
+        // insert test data
+        s.execute("create table t1 (i int)");
+        s.execute("create table t2 (j int)");
+        s.execute("insert into t1 values 1, 5, 7, 11, 13, 17, 19");
+        s.execute("insert into t2 values 23, 29, 31, 37, 43, 47, 53");
+        s.execute("insert into t1 select 23 * i from t1 where i < 19");
+        s.execute("insert into t2 select 23 * j from t2 where j < 55");
+
+        // enable runtime statistics
+        s.execute("call syscs_util.syscs_set_runtimestatistics(1)");
+
+        // Following will show two qualifiers for T2 and three for T1
+        // because transitive closure adds two new qualifiers, "t2.j >= 23"
+        // and "t1.i <= 30" to the list.
+        JDBC.assertSingleValueResultSet(
+                s.executeQuery(
+                    "select i from t1, t2 where " +
+                    "t1.i = t2.j and t1.i >= 23 and t2.j <= 30"),
+                "23");
+
+        List expectedOperators = Arrays.asList(new String[] {
+                    "Operator: <", "Operator: <=",
+                    "Operator: <", "Operator: <=", "Operator: ="
+                });
+
+        assertEquals(expectedOperators, extractOperators(getStatistics()));
+
+        // But if we put the constants on the left-hand side, we didn't
+        // detect the transitive closure and thus we had a single qualifier
+        // for T2 and only two qualifiers for T1.
+        JDBC.assertSingleValueResultSet(
+                s.executeQuery(
+                    "select i from t1, t2 where " +
+                    "t1.i = t2.j and 23 <= t1.i and 30 >= t2.j"),
+                "23");
+
+        // Verify that we now have all the expected qualifiers.
+        assertEquals(expectedOperators, extractOperators(getStatistics()));
+    }
+
+    /**
+     * Get the runtime statistics for the previous statement executed on the
+     * default connection (if collection of runtime statistics has been
+     * enabled).
+     *
+     * @return a string with the runtime statistics
+     */
+    private String getStatistics() throws SQLException {
+        ResultSet rs = createStatement().executeQuery(
+                "values syscs_util.syscs_get_runtimestatistics()");
+        rs.next();
+        String stats = rs.getString(1);
+        JDBC.assertEmpty(rs);
+        return stats;
+    }
+
+    /**
+     * Extract all the operators from the runtime statistics.
+     *
+     * @param stats the runtime statistics
+     * @return a list of all operators
+     */
+    private List extractOperators(String stats) throws IOException {
+        ArrayList ops = new ArrayList();
+        BufferedReader r = new BufferedReader(new StringReader(stats));
+        String line;
+        while ((line = r.readLine()) != null) {
+            line = line.trim();
+            if (line.startsWith("Operator: ")) {
+                ops.add(line);
+            }
+        }
+        return ops;
+    }
+}

Propchange: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/PredicateTest.java
------------------------------------------------------------------------------
    svn:eol-style = native

Modified: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/_Suite.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/_Suite.java?rev=887156&r1=887155&r2=887156&view=diff
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/_Suite.java
(original)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/_Suite.java
Fri Dec  4 11:02:16 2009
@@ -77,6 +77,7 @@
         suite.addTest(JoinTest.suite());
 		suite.addTest(LangScripts.suite());
         suite.addTest(MathTrigFunctionsTest.suite());
+        suite.addTest(PredicateTest.suite());
         suite.addTest(PrepareExecuteDDL.suite());
         suite.addTest(ReferentialActionsTest.suite());
         suite.addTest(RolesTest.suite());



Mime
View raw message