You are viewing a plain text version of this content. The canonical link for it is here.
Posted to derby-commits@db.apache.org by ka...@apache.org on 2009/12/04 12:02:17 UTC
svn commit: r887156 - in /db/derby/code/trunk/java:
engine/org/apache/derby/impl/sql/compile/
testing/org/apache/derbyTesting/functionTests/tests/lang/
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());