Index: java/engine/org/apache/derby/impl/sql/compile/Predicate.java =================================================================== --- java/engine/org/apache/derby/impl/sql/compile/Predicate.java (revision 604507) +++ java/engine/org/apache/derby/impl/sql/compile/Predicate.java (working copy) @@ -1195,6 +1195,12 @@ { // Left operand is the scoped operand. ((ColumnReference)operand).remapColumnReferences(); + + if (binRelOp.isInListProbeNode()) + { + ((ColumnReference)binRelOp.getInListOp().getLeftOperand()) + .remapColumnReferences(); + } } else { Index: java/engine/org/apache/derby/impl/sql/compile/BinaryRelationalOperatorNode.java =================================================================== --- java/engine/org/apache/derby/impl/sql/compile/BinaryRelationalOperatorNode.java (revision 604507) +++ java/engine/org/apache/derby/impl/sql/compile/BinaryRelationalOperatorNode.java (working copy) @@ -36,6 +36,8 @@ import org.apache.derby.iapi.sql.compile.C_NodeTypes; import org.apache.derby.iapi.sql.compile.Optimizable; +import org.apache.derby.iapi.sql.compile.Visitable; +import org.apache.derby.iapi.sql.compile.Visitor; import org.apache.derby.iapi.sql.dictionary.ConglomerateDescriptor; @@ -160,6 +162,117 @@ return inListProbeSource; } + /** + * @see BinaryOperatorNode#bindExpression + */ + public ValueNode bindExpression(FromList fromList, + SubqueryList subqueryList, java.util.Vector aggregateVector) + throws StandardException + { + if (isInListProbeNode()) + { + /* This node is a "probe predicate" that was created to replace + * an InListOperatorNode. So if we ever get here (will we?), then + * the InListOperatorNode has to be re-binded. In that case we + * play it safe and "start over" with the original InListOpNode + * again, because the re-binding could theoretically change + * something on which this "probe predicate" depends. Note that + * this should only ever happen if a call is made to this method + * (bindExpresssion()) on a node that has already been bound and + * preprocessed once (we know this because a "probe predicate" + * cannot exist until preprocessing has occurred on the target + * InListOperatorNode). While seemingly unlikely, there don't + * appear to be any rules saying that a bindExpression can't + * happen after a preprocess(), so we code for it just in case... + */ + System.out.println("Whoa, re-bound probe predicate after all!"); + return inListProbeSource.bindExpression( + fromList, subqueryList, aggregateVector); + } + + return super.bindExpression(fromList, subqueryList, aggregateVector); + } + + /** + * @see BinaryOperatorNode#preprocess + */ + public ValueNode preprocess(int numTables, FromList outerFromList, + SubqueryList outerSubqueryList, PredicateList outerPredicateList) + throws StandardException + { + if (isInListProbeNode()) + { + /* This node is a "probe predicate" that was created to replace + * an InListOperatorNode. So if we ever get here (will we?), then + * the InListOperatorNode has to be re-preprocessed. In that + * case this probe predicate may no longer be valid, so we start + * over with a clean pre-process call again. Note that this should + * if a call is made to preprocess a node that has already been + * preprocessed once (we know this because a "probe predicate" + * cannot exist until preprocessing has occurred on the target + * InListOperatorNode). While seemingly unlikely, there don't + * appear to be any rules saying that a preprocess() call can't + * happen more than once on a single node, so we code for it + * just in case... + */ + System.out.println("Whoa, re-preprocess of probe predicate!"); + return inListProbeSource.preprocess(numTables, + outerFromList, outerSubqueryList, outerPredicateList); + } + + return super.preprocess(numTables, + outerFromList, outerSubqueryList, outerPredicateList); + } + + + /** + * Set the leftOperand to the specified ValueNode + * + * @param newLeftOperand The new leftOperand + */ + public void setLeftOperand(ValueNode newLeftOperand) + { + super.setLeftOperand(newLeftOperand); + + /* If this relational operator constitutes a "probe predicate" then + * we have to make sure we update the underlying InListOperatorNode, + * as well. This is because there's still the possibility that we'll + * find the probe predicate to be "not useful" and thus have to + * "revert" back to the InListOperatorNode at code generation time. + * If we do that see (BinaryOperatorNode.generateExpression()) then + * the InListOperatorNode must have an up-to-date left operand, as + * well. + */ + if (isInListProbeNode()) + inListProbeSource.setLeftOperand(newLeftOperand); + } + + /** + * @see BinaryOperatorNode#remapColumnReferencesToExpressions() + */ + public ValueNode remapColumnReferencesToExpressions() + throws StandardException + { + ValueNode retNode = super.remapColumnReferencesToExpressions(); + + /* If this relational operator constitutes a "probe predicate" then + * we have to make sure we update the underlying InListOperatorNode, + * as well. This is because there's still the possibility that we'll + * find the probe predicate to be "not useful" and thus have to + * "revert" back to the InListOperatorNode at code generation time. + * If we do that see (BinaryOperatorNode.generateExpression()) then + * the InListOperatorNode must have an up-to-date left operand, as + * well. + */ + if (isInListProbeNode()) + { + inListProbeSource = (InListOperatorNode) + inListProbeSource.remapColumnReferencesToExpressions(); + } + + return retNode; + } + /** @see RelationalOperator#getColumnOperand */ public ColumnReference getColumnOperand( Optimizable optTable, @@ -512,6 +625,37 @@ } /** + * Accept a visitor, and call v.visit() + * on child nodes as necessary. + * + * @param v the visitor + * + * @exception StandardException on error + */ + public Visitable accept(Visitor v) + throws StandardException + { + Visitable returnNode = super.accept(v); + + /* If this relational operator constitutes a "probe predicate" then + * we have to make sure we update the underlying InListOperatorNode, + * as well. This is because there's still the possibility that we'll + * find the probe predicate to be "not useful" and thus have to + * "revert" back to the InListOperatorNode at code generation time. + * If we do that see (BinaryOperatorNode.generateExpression()) then + * the InListOperatorNode must have an up-to-date left operand, as + * well. + */ + if (isInListProbeNode() && !v.stopTraversal()) + { + inListProbeSource = + (InListOperatorNode)inListProbeSource.accept(v); + } + + return returnNode; + } + + /** * Return true if a key column for the given table is found on the * left side of this operator, false if it is found on the right * side of this operator. Index: java/testing/org/apache/derbyTesting/functionTests/tests/lang/InListMultiProbeTest.java =================================================================== --- java/testing/org/apache/derbyTesting/functionTests/tests/lang/InListMultiProbeTest.java (revision 604507) +++ java/testing/org/apache/derbyTesting/functionTests/tests/lang/InListMultiProbeTest.java (working copy) @@ -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 { @@ -499,6 +498,54 @@ } /** + * 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(); + } + + /** * Insert the received number of rows into DATA_TABLE via * batch processing. */