svn commit: r890370 - in /db/derby/code/trunk/java: engine/org/apache/derby/impl/sql/compile/PredicateList.java testing/org/apache/derbyTesting/functionTests/tests/lang/PredicateTest.java

[ka...@apache.org]

Author: kahatlen
Date: Mon Dec 14 15:35:06 2009
New Revision: 890370

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

Extended the fix to work on parameters in addition to constants.

Modified:
    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/PredicateTest.java

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=890370&r1=890369&r2=890370&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 Mon Dec 14 15:35:06 2009
@@ -1440,6 +1440,17 @@
 		}
 	}
 
+    /**
+     * Check if a node is representing a constant or a parameter.
+     *
+     * @param node the node to check
+     * @return {@code true} if the node is a constant or a parameter, {@code
+     * false} otherwise
+     */
+    private static boolean isConstantOrParameterNode(ValueNode node) {
+        return node instanceof ConstantNode || node instanceof ParameterNode;
+    }
+
 	/**
 	 * Push all predicates, which can be pushed, into the underlying select.
 	 * A predicate can be pushed into an underlying select if the source of 
@@ -1510,8 +1521,7 @@
 					opNode = (BinaryRelationalOperatorNode) andNode.getLeftOperand();
 					// Investigate using invariant interface to check rightOperand
 					if (! (opNode.getLeftOperand() instanceof ColumnReference) ||
-					    ! (opNode.getRightOperand() instanceof ConstantNode ||
-							 opNode.getRightOperand() instanceof ParameterNode))
+						! isConstantOrParameterNode(opNode.getRightOperand()))
 						continue;
 
 					crNode = (ColumnReference) opNode.getLeftOperand();
@@ -2273,12 +2283,13 @@
 
 				// RESOLVE: Consider using variant type of the expression, instead of
 				// ConstantNode or ParameterNode in the future.
-				if (left instanceof ColumnReference && 
-					  (right instanceof ConstantNode || right instanceof ParameterNode))
+				if (left instanceof ColumnReference &&
+						isConstantOrParameterNode(right))
 				{
 					searchClauses.addElement(predicate);
 				}
-				else if (left instanceof ConstantNode && right instanceof ColumnReference)
+				else if (isConstantOrParameterNode(left) &&
+						right instanceof ColumnReference)
 				{
 					// put the ColumnReference on the left to simplify things
 					andNode.setLeftOperand(bcon.getSwappedEquivalent());

Modified: 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=890370&r1=890369&r2=890370&view=diff
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/PredicateTest.java (original)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/PredicateTest.java Mon Dec 14 15:35:06 2009
@@ -24,6 +24,7 @@
 import java.io.BufferedReader;
 import java.io.IOException;
 import java.io.StringReader;
+import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.sql.Statement;
@@ -98,6 +99,29 @@
 
         // Verify that we now have all the expected qualifiers.
         assertEquals(expectedOperators, extractOperators(getStatistics()));
+
+        // Now check that we get the same plan with parameters instead of
+        // constants on the right-hand side.
+
+        PreparedStatement paramRight = prepareStatement(
+                "select i from t1, t2 where " +
+                "t1.i = t2.j and t1.i >= ? and t2.j <= ?");
+        paramRight.setInt(1, 23);
+        paramRight.setInt(2, 30);
+
+        JDBC.assertSingleValueResultSet(paramRight.executeQuery(), "23");
+        assertEquals(expectedOperators, extractOperators(getStatistics()));
+
+        // Same plan expected with parameters on the left-hand side.
+
+        PreparedStatement paramLeft = prepareStatement(
+                "select i from t1, t2 where " +
+                "t1.i = t2.j and ? <= t1.i and ? >= t2.j");
+        paramLeft.setInt(1, 23);
+        paramLeft.setInt(2, 30);
+
+        JDBC.assertSingleValueResultSet(paramLeft.executeQuery(), "23");
+        assertEquals(expectedOperators, extractOperators(getStatistics()));
     }
 
     /**