db-derby-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From abr...@apache.org
Subject svn commit: r605616 - in /db/derby/code/trunk/java: engine/org/apache/derby/impl/sql/compile/ testing/org/apache/derbyTesting/functionTests/tests/lang/
Date Wed, 19 Dec 2007 16:53:30 GMT
Author: abrown
Date: Wed Dec 19 08:53:29 2007
New Revision: 605616

URL: http://svn.apache.org/viewvc?rev=605616&view=rev
Log:
DERBY-3253: Fix NPE for IN list operator when the probe predicate is
pushed into a subselect but then multi-probing does not occur.

Modified:
    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/OrNode.java
    db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/InListMultiProbeTest.java

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=605616&r1=605615&r2=605616&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
Wed Dec 19 08:53:29 2007
@@ -80,9 +80,12 @@
 	 * this BinRelOp is for an IN-list probe predicate; 2) if the
 	 * optimizer chooses a plan for which the probe predicate is
 	 * not usable as a start/stop key then we'll "revert" the pred
-	 * back to the InListOperatorNode referenced here.
+	 * back to the InListOperatorNode referenced here.  NOTE: Once
+	 * set, this variable should *only* ever be accessed via the
+	 * isInListProbeNode() or getInListOp() methods--see comments
+	 * in the latter method for more.
 	 */
-	InListOperatorNode inListProbeSource = null;
+	private InListOperatorNode inListProbeSource = null;
 
 	public void init(Object leftOperand, Object rightOperand)
 	{
@@ -153,10 +156,46 @@
 	 * If this rel op was created for an IN-list probe predicate then return
 	 * the underlying InListOperatorNode.  Will return null if this rel
 	 * op is a "legitimate" relational operator (as opposed to a disguised
-	 * IN-list).
+	 * IN-list).  With the exception of nullability checking via the
+	 * isInListProbeNode() method, all access to this.inListProbeSource
+	 * MUST come through this method, as this method ensures that the
+	 * left operand of the inListProbeSource is set correctly before
+	 * returning it.
 	 */
 	protected InListOperatorNode getInListOp()
 	{
+		if (inListProbeSource != null)
+		{
+			/* Depending on where this probe predicate currently sits
+			 * in the query tree, this.leftOperand *may* have been
+			 * transformed, replaced, or remapped one or more times
+			 * since inListProbeSource was last referenced. Since the
+			 * leftOperand of the IN list should be the same regardless
+			 * of which "version" of the operation we're looking at
+			 * (i.e. the "probe predicate" version (this node) vs the
+			 * original version (inListProbeSource)), we have to make
+			 * sure that all of the changes made to this.leftOperand
+			 * are reflected in inListProbeSource's leftOperand, as
+			 * well.  In doing so we ensure the caller of this method
+			 * will see an up-to-date version of the InListOperatorNode--
+			 * and thus, if the caller references the InListOperatorNode's
+			 * leftOperand, it will see the correct information. One
+			 * notable example of this is at code generation time, where
+			 * if this probe predicate is deemed "not useful", we'll
+			 * generate the underlying InListOperatorNode instead of
+			 * "this".  For that to work correctly, the InListOperatorNode
+			 * must have the correct leftOperand. DERBY-3253.
+			 *
+			 * That said, since this.leftOperand will always be "up-to-
+			 * date" w.r.t. the current query tree (because this probe
+			 * predicate sits in the query tree and so all relevant
+			 * transformations will be applied here), the simplest way
+			 * to ensure the underlying InListOperatorNode also has an
+			 * up-to-date leftOperand is to set it to this.leftOperand.
+			 */
+			inListProbeSource.setLeftOperand(this.leftOperand);
+		}
+
 		return inListProbeSource;
 	}
 
@@ -777,7 +816,7 @@
 		 * the IN-list).  That would lead to wrong results (missing rows)
 		 * because that restriction is incorrect.
 		 */
-		if (inListProbeSource != null)
+		if (isInListProbeNode())
 			return false;
 
 		FromTable	ft;
@@ -1215,7 +1254,7 @@
 		 * it a "relational operator"; it's actually a disguised IN-list
 		 * operator.
 		 */
-		return (inListProbeSource == null);
+		return !isInListProbeNode();
 	}
 	
 	/** @see ValueNode#isBinaryEqualsOperatorNode */
@@ -1225,11 +1264,18 @@
 		 * it as an "equals operator"; it's actually a disguised IN-list
 		 * operator.
 		 */
-		return (inListProbeSource == null) &&
+		return !isInListProbeNode() &&
 			(operatorType == RelationalOperator.EQUALS_RELOP);
 	}
 
-	/** @see ValueNode#isInListProbeNode */
+	/**
+	 * @see ValueNode#isInListProbeNode
+	 *
+	 * It's okay for this method to reference inListProbeSource directly
+	 * because it does not rely on the contents of inListProbeSource's
+	 * leftOperand, and a caller of this method cannot gain access to
+	 * inListProbeSource's leftOperand through this method.
+	 */
 	public boolean isInListProbeNode()
 	{
 		return (inListProbeSource != null);
@@ -1247,7 +1293,7 @@
 		/* If this rel op is for a probe predicate then we do not treat
 		 * it as an equality node; it's actually a disguised IN-list node.
 		 */
-		if (inListProbeSource != null)
+		if (isInListProbeNode())
 			return false;
 
 		ColumnReference cr = getColumnOperand(optTable,

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/OrNode.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/OrNode.java?rev=605616&r1=605615&r2=605616&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/OrNode.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/OrNode.java Wed Dec
19 08:53:29 2007
@@ -152,7 +152,7 @@
 						if (left instanceof BinaryRelationalOperatorNode)
 						{
  							bron = (BinaryRelationalOperatorNode)left;
-							if (bron.getInListOp() == null)
+							if (!bron.isInListProbeNode())
 							{
 								SanityManager.THROWASSERT(
 								"isRelationalOperator() unexpectedly returned "
@@ -223,7 +223,7 @@
 					OrNode on = (OrNode) vn;
 					BinaryRelationalOperatorNode bron =
 						(BinaryRelationalOperatorNode) on.getLeftOperand();
-					if (bron.getInListOp() != null)
+					if (bron.isInListProbeNode())
 					{
 						/* If we have an OR between multiple IN-lists on the same
 						 * column then just combine them into a single IN-list.

Modified: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/InListMultiProbeTest.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/InListMultiProbeTest.java?rev=605616&r1=605615&r2=605616&view=diff
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/InListMultiProbeTest.java
(original)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/InListMultiProbeTest.java
Wed Dec 19 08:53:29 2007
@@ -212,11 +212,10 @@
     }
 
     /**
-     * The one test fixture for this test.  Executes three different types
-     * of queries ("strategies") repeatedly with an increasing number of
-     * values in the IN list.  Underneath we will check the query plan
-     * for each query to make sure that Derby is doing multi-probing as
-     * expected.
+     * Executes three different types of queries ("strategies") repeatedly
+     * with an increasing number of values in the IN list.  Underneath we
+     * will check the query plan for each query to make sure that Derby is
+     * doing multi-probing as expected.
      */
     public void testMultiProbing() throws Exception
     {
@@ -495,6 +494,54 @@
         st.execute("drop table mytable");
 
         ps.close();
+        st.close();
+    }
+
+    /**
+     * Test the scenario in which Derby creates an IN-list probe
+     * predicate, remaps its left operand to point to a nested
+     * SELECT query, and then decides to *not* use the probe
+     * predicate in the final plan.  The remapping of the left
+     * operand will cause the probe predicate's left operand to
+     * be set to a different ColumnReference object--one that
+     * points to the target table in the subselect.  Then when
+     * the optimizer decides to *not* use the probe predicate
+     * in the final query, we'll revert back to the original IN
+     * list (InListOperatorNode) and generate that for the query.
+     * When we do so, the left operand of the InListOperatorNode
+     * must reflect the fact that the IN operation's left operand
+     * has changed (it now points to the table from the subselect).
+     * Otherwise the InListOperatorNode will generate an invalid
+     * ColumnReference.  DERBY-3253.
+     */
+    public void testProbePredPushedIntoSelectThenReverted()
+        throws Exception
+    {
+        Statement st = createStatement();
+
+        st.execute("create table d3253 (i int, vc varchar(10))");
+        st.execute("insert into d3253 values " +
+            "(1, 'one'), (2, 'two'), (3, 'three'), (1, 'un')");
+
+        /* Before DERBY-3253 was fixed, this query would have thrown
+         * an execution time NPE due to the fact the generated column
+         * reference was pointing to the wrong place.
+         */
+        JDBC.assertUnorderedResultSet(st.executeQuery(
+            "select x.* from d3253, (select * from d3253) x " +
+            "where d3253.i = x.i and x.vc in ('un', 'trois')"),
+            new String [][] {{"1","un"},{"1","un"}});
+
+        JDBC.assertUnorderedResultSet(st.executeQuery(
+            "select x.* from d3253, (select * from d3253) x " +
+            "where d3253.i = x.i and x.i in (2, 3)"),
+            new String [][] {{"2","two"},{"3","three"}});
+
+        JDBC.assertEmpty(st.executeQuery(
+            "select x.* from d3253, (select * from d3253) x " +
+            "where d3253.i = x.i and x.vc in ('uno', 'tres')"));
+
+        st.execute("drop table d3253");
         st.close();
     }
 



Mime
View raw message