db-derby-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kahat...@apache.org
Subject svn commit: r816536 - in /db/derby/code/trunk/java: engine/org/apache/derby/impl/sql/execute/ testing/org/apache/derbyTesting/functionTests/tests/lang/
Date Fri, 18 Sep 2009 08:46:30 GMT
Author: kahatlen
Date: Fri Sep 18 08:46:29 2009
New Revision: 816536

URL: http://svn.apache.org/viewvc?rev=816536&view=rev
Log:
DERBY-4376: Simple select runs forever

A MultiProbeTableScanResultSet performes one index scan per distinct
element in the IN list. Changing the start and stop positions of the
underlying index scan, and moving to the next element in the IN list,
happens when the scan controller is reopened. However, the reopening
of the scan controller is controlled by TableScanResultSet, which
skips the reopening if the current start and stop positions are
guaranteed to make the scan return zero rows. Basically, if the
start/stop positions are initialized with NULLs, the scan is not
reopened, and we do not move to the next element of the IN list. We
therefore just keep on retrying at the same position in the IN list,
and the query execution is stuck forever.

This patch moves all the multi-probe logic out of TableScanResultSet
and into MultiProbeTableScanResultSet, and instead adds methods that
MPTSRS can override where different logic is needed. In short:

1) Initialization of the start/stop keys has been factored out of
TSRS.openCore()/TSRS.reopenCore() into a helper method
initStartAndStopKey().

2) MPTSRS overrides initStartAndStopKey() and updates the keys with
the actual probe value. This ensures that the keys now have the
correct values when TSRS.reopenCore() calls skipScan().

3) Methods in TSRS that take a probeValue argument have been removed,
since MPTSRS.initStartAndStopKey() now does all the work with
retrieving the probe value and updating the keys.

4) Since the next probe value is fetched earlier now
MPTSRS.reopenScanController() can no longer use the return value from
getNextProbeValue() to decide whether or not it should be a no-op (it
should be a no-op if the next probe value was null, which means that
the probe list has been exhausted). To ensure that we don't open scans
in those cases, set a flag in initStartAndStopKey() to indicate
whether or not the probe list was exhausted, and override skipScan()
to check that flag and return true if no new probe value was found.

Modified:
    db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/execute/MultiProbeTableScanResultSet.java
    db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/execute/TableScanResultSet.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/execute/MultiProbeTableScanResultSet.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/execute/MultiProbeTableScanResultSet.java?rev=816536&r1=816535&r2=816536&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/execute/MultiProbeTableScanResultSet.java
(original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/execute/MultiProbeTableScanResultSet.java
Fri Sep 18 08:46:29 2009
@@ -28,11 +28,11 @@
 
 import org.apache.derby.iapi.store.access.Qualifier;
 import org.apache.derby.iapi.store.access.StaticCompiledOpenConglomInfo;
-import org.apache.derby.iapi.store.access.TransactionController;
 
 import org.apache.derby.iapi.sql.Activation;
 import org.apache.derby.iapi.sql.compile.RowOrdering;
 import org.apache.derby.iapi.sql.execute.CursorResultSet;
+import org.apache.derby.iapi.sql.execute.ExecIndexRow;
 import org.apache.derby.iapi.sql.execute.ExecRow;
 
 import org.apache.derby.iapi.types.DataValueDescriptor;
@@ -89,6 +89,14 @@
     private int sortRequired;
 
     /**
+     * Tells whether or not we should skip the next attempt to (re)open the
+     * scan controller. If it is {@code true} it means that the previous call
+     * to {@link #initStartAndStopKey()} did not find a new probe value, which
+     * means that the probe list is exhausted and we shouldn't perform a scan.
+     */
+    private boolean skipNextScan;
+
+    /**
      * Constructor.  Just save off the relevant probing state and pass
      * everything else up to TableScanResultSet.
      * 
@@ -215,29 +223,6 @@
     }
 
     /**
-     * Open the scan controller
-     *
-     * @param tc transaction controller; will open one if null.
-     * @exception StandardException thrown on failure to open
-     */
-    protected void openScanController(TransactionController tc)
-        throws StandardException
-    {
-        /* If we're opening the scan controller for the first time then
-         * we want to use the first value in the (now sorted) list as
-         * the start/stop key.  That's what we pass in here.
-         */
-        openScanController(tc, probeValues[0]);
-
-        /* probeValIndex should be the index of the *next* value to
-         * use.  Since we just positioned ourselves at the 0th probe
-         * value with the above call, the next value we want is the
-         * one at index "1".
-         */
-        probeValIndex = 1;
-    }
-
-    /**
      * @see NoPutResultSet#reopenCore
      */
     public void reopenCore() throws StandardException
@@ -298,6 +283,33 @@
      */
     protected void reopenScanController() throws StandardException
     {
+        // TableScanResultSet.reopenScanController() will reset rowsThisScan
+        // because it thinks this is a completely new scan. However, we want
+        // it to reflect the total number of rows seen in the multi-probe
+        // scan, so we keep the original value and restore it after reopening
+        // the controller. Instead, we reset rowsThisScan to 0 each time
+        // initStartAndStopKey() is called on the first probe value.
+        long rows = rowsThisScan;
+        super.reopenScanController();
+        rowsThisScan = rows;
+    }
+
+    /**
+     * Initialize the start key and the stop key used in the scan. Both keys
+     * will be set to the probe value. If no new probe value was found (the
+     * probe list was exhausted), the flag skipNextScan will be {@code true}
+     * when the method returns to prevent a new scan from being reopened with
+     * a missing or incorrect probe value.
+     */
+    void initStartAndStopKey() throws StandardException {
+
+        // Make sure the fields are initialized with a placeholder.
+        // startPosition and stopPosition will always be non-null in a
+        // MultiProbeTableScanResultSet, and they will always be initialized
+        // to the first value in the probe list. They will be changed to
+        // the actual probe value later in this method.
+        super.initStartAndStopKey();
+
         /* If we're looking for the first value in the probe list, then
          * reset the row scan count.  Otherwise leave it unchanged since
          * we're just continuing an already-opened scan.  Note that we
@@ -307,21 +319,76 @@
         if (probeValIndex == 0)
             rowsThisScan = 0;
 
-        DataValueDescriptor pv = null;
-        if (moreInListVals())
-        {
-            pv = getNextProbeValue();
-            if (pv == null)
-            {
-                /* We'll get here when we've exhausted the probe list. In
-                 * that case leave the scan as it is, which effectively
-                 * means we are done.
-                 */
-                return;
+        DataValueDescriptor[] startPositionRow = startPosition.getRowArray();
+        DataValueDescriptor[] stopPositionRow = stopPosition.getRowArray();
+
+        DataValueDescriptor probeValue = getNextProbeValue();
+
+		/* If we have a probe value then we do the "probe" by positioning
+		 * the scan at the first row matching the value.  The way to do
+		 * that is to use the value as a start key, which is what will
+		 * happen if we plug it into first column of "startPositionRow".
+		 * So in this case startPositionRow[0] functions as a "place-holder"
+		 * for the probe value.  The same goes for stopPositionRow[0].
+		 *
+		 * Note that it *is* possible for a start/stop key to contain more
+		 * than one column (ex. if we're scanning a multi-column index). In
+		 * that case we plug probeValue into the first column of the start
+		 * and/or stop key and leave the rest of the key as it is.  As an
+		 * example, assume we have the following predicates:
+		 *
+		 *    ... where d in (1, 20000) and b > 200 and b <= 500
+		 *
+		 * And assume further that we have an index defined on (d, b).
+		 * In this case it's possible that we have TWO start predicates
+		 * and TWO stop predicates: the IN list will give us "d = probeVal",
+		 * which is a start predicate and a stop predicate; then "b > 200"
+		 * may give us a second start predicate, while "b <= 500" may give
+		 * us a second stop predicate.  So in this situation we want our
+		 * start key to be:
+		 *
+		 *    (probeValue, 200)
+		 *
+		 * and our stop key to be:
+		 *
+		 *    (probeValue, 500).
+		 *
+		 * This will effectively limit the scan so that it only returns
+		 * rows whose "D" column equals probeValue and whose "B" column
+		 * falls in the range of 200 thru 500.
+		 *
+		 * Note: Derby currently only allows a single start/stop predicate
+		 * per column. See PredicateList.orderUsefulPredicates().
+		 */
+        if (probeValue != null) {
+            startPositionRow[0] = probeValue;
+            if (!sameStartStopPosition) {
+                stopPositionRow[0] = startPositionRow[0];
             }
         }
 
-        reopenScanController(pv);
+        // If we didn't find a new probe value, the probe list is exhausted,
+        // and we shouldn't open a new scan. skipScan() will detect this and
+        // prevent (re)openScanController() from being called.
+        skipNextScan = (probeValue == null);
+    }
+
+    /**
+     * Check if the scan should be skipped. It should be skipped if (1)
+     * {@link #initStartAndStopKey()} exhausted the probe list, or (2) the scan
+     * should return no results because of nulls in the start key or stop key.
+     * See {@link NoPutResultSetImpl#skipScan(ExecIndexRow,ExecIndexRow)} for
+     * details about (2).
+     *
+     * @param startPosition the key on which to start the scan
+     * @param stopPosition the key on which to stop the scan
+     * @return {@code true} if scan should be skipped, {@code false} otherwise
+     */
+    protected boolean skipScan(
+            ExecIndexRow startPosition, ExecIndexRow stopPosition)
+		throws StandardException
+    {
+        return skipNextScan || super.skipScan(startPosition, stopPosition);
     }
 
     /**

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/execute/TableScanResultSet.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/execute/TableScanResultSet.java?rev=816536&r1=816535&r2=816536&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/execute/TableScanResultSet.java
(original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/execute/TableScanResultSet.java
Fri Sep 18 08:46:29 2009
@@ -82,7 +82,7 @@
     protected int[] indexCols;		//index keys base column position array
 	public int rowsPerRead;
 	public boolean forUpdate;
-	private boolean sameStartStopPosition;
+	final boolean sameStartStopPosition;
 	private boolean nextDone;
 	private RowLocation rlTemplate;
 
@@ -244,19 +244,7 @@
 		if (dcoci == null)
 			dcoci = tc.getDynamicCompiledConglomInfo(conglomId);
 
-
-		if (startKeyGetter != null)
-		{
-			startPosition = (ExecIndexRow) startKeyGetter.invoke(activation);
-			if (sameStartStopPosition)
-			{
-				stopPosition = startPosition;
-			}
-		}
-		if (stopKeyGetter != null)
-		{
-			stopPosition = (ExecIndexRow) stopKeyGetter.invoke(activation);
-		}
+        initStartAndStopKey();
 
 		/* NOTE: We always open the ScanController on the 1st open
 		 * to do the keyed conglomerate check.
@@ -327,6 +315,22 @@
 		openTime += getElapsedMillis(beginTime);
 	}
 
+    /**
+     * Initialize the {@code startPosition} and {@code stopPosition} fields
+     * which are used to limit the rows returned by the scan.
+     */
+    void initStartAndStopKey() throws StandardException {
+        if (startKeyGetter != null) {
+            startPosition = (ExecIndexRow) startKeyGetter.invoke(activation);
+            if (sameStartStopPosition) {
+                stopPosition = startPosition;
+            }
+        }
+        if (stopKeyGetter != null) {
+            stopPosition = (ExecIndexRow) stopKeyGetter.invoke(activation);
+        }
+    }
+
 	/*
 	** Open the scan controller
 	**
@@ -335,75 +339,11 @@
 	protected void openScanController(TransactionController tc)
 		throws StandardException
 	{
-		openScanController(tc, (DataValueDescriptor)null);
-	}
-
-	/*
-	** Does the work of openScanController.
-	**
-	** @param tc transaction controller; will open one if null.
-	** @param probeValue If non-null then we will open the scan controller
-	**  and position it using the received probeValue as the start key.
-	**  Otherwise we'll use whatever value is in startPosition (if non-
-	**  null) as the start key.
-	*/
-	protected void openScanController(TransactionController tc,
-		DataValueDescriptor probeValue) throws StandardException
-	{
 		DataValueDescriptor[] startPositionRow = 
             startPosition == null ? null : startPosition.getRowArray();
 		DataValueDescriptor[] stopPositionRow = 
             stopPosition == null ? null : stopPosition.getRowArray();
 
-		/* If we have a probe value then we do the "probe" by positioning
-		 * the scan at the first row matching the value.  The way to do
-		 * that is to use the value as a start key, which is what will
-		 * happen if we plug it into first column of "startPositionRow".
-		 * So in this case startPositionRow[0] functions as a "place-holder"
-		 * for the probe value.  The same goes for stopPositionRow[0].
-		 *
-		 * Note that it *is* possible for a start/stop key to contain more
-		 * than one column (ex. if we're scanning a multi-column index). In
-		 * that case we plug probeValue into the first column of the start
-		 * and/or stop key and leave the rest of the key as it is.  As an 
-		 * example, assume we have the following predicates:
-		 *
-		 *    ... where d in (1, 20000) and b > 200 and b <= 500
-		 *
-		 * And assume further that we have an index defined on (d, b).
-		 * In this case it's possible that we have TWO start predicates
-		 * and TWO stop predicates: the IN list will give us "d = probeVal",
-		 * which is a start predicate and a stop predicate; then "b > 200"
-		 * may give us a second start predicate, while "b <= 500" may give
-		 * us a second stop predicate.  So in this situation we want our
-		 * start key to be:
-		 *
-		 *    (probeValue, 200)
-		 *
-		 * and our stop key to be:
-		 *
-		 *    (probeValue, 500).
-		 *
-		 * This will effectively limit the scan so that it only returns
-		 * rows whose "D" column equals probeValue and whose "B" column
-		 * falls in the range of 200 thru 500.
-		 *
-		 * Note: Derby currently only allows a single start/stop predicate
-		 * per column. See PredicateList.orderUsefulPredicates().
-		 */
-		if (probeValue != null)
-		{
-			startPositionRow[0] = probeValue;
-
-		 	/* If the start key and stop key are the same, we've already set
-			 * stopPosition equal to startPosition as part of openCore().
-			 * So by putting the probe value into startPositionRow[0], we
-			 * also put it into stopPositionRow[0].
-			 */
-			if (!sameStartStopPosition)
-				stopPositionRow[0] = probeValue;
-		}
-
 		// Clear the Qualifiers's Orderable cache 
 		if (qualifiers != null)
 		{
@@ -461,44 +401,12 @@
 	*/
 	protected void reopenScanController() throws StandardException
 	{
-		reopenScanController((DataValueDescriptor)null);
-	}
-
-	/*
-	** Does the work of reopenScanController.
-	**
-	** @param probeValue If non-null then we will open the scan controller
-	**  and position it using the received probeValue as the start key.
-	**  Otherwise we'll use whatever value is in startPosition (if non-
-	**  null) as the start key.
-	*/
-	protected void reopenScanController(DataValueDescriptor probeValue)
-		throws StandardException
-	{
 		DataValueDescriptor[] startPositionRow = 
             startPosition == null ? null : startPosition.getRowArray();
 		DataValueDescriptor[] stopPositionRow = 
             stopPosition == null ? null : stopPosition.getRowArray();
 
-		/* If we have a probe value then we do the "probe" by using the
-		 * value as a start and stop key.  See openScanController() for
-		 * details.  Note that in this case we do *not* want to reset
-		 * the rowsThisScan variable because we are going to be doing
-		 * multiple "probes" for a single scan.  Logic to detect when
-		 * when we've actually started a new scan (as opposed to just
-		 * repositioning an existing scan based on a probe value) is
-		 * in MultiProbeTableScanResultSet.reopenScanController(),
-		 * and that method will then take care of resetting the variable
-		 * (if needed) for probing scans.
-		 */
-		if (probeValue != null)
-		{
-			startPositionRow[0] = probeValue;
-			if (!sameStartStopPosition)
-				stopPositionRow[0] = probeValue;
-		}
-		else
-			rowsThisScan = 0;
+        rowsThisScan = 0;
 
 		// Clear the Qualifiers's Orderable cache 
 		if (qualifiers != null)
@@ -531,18 +439,7 @@
 		if (SanityManager.DEBUG)
 		    SanityManager.ASSERT(isOpen, "TableScanResultSet not open, cannot reopen");
 
-		if (startKeyGetter != null)
-		{
-			startPosition = (ExecIndexRow) startKeyGetter.invoke(activation);
-			if (sameStartStopPosition)
-			{
-				stopPosition = startPosition;
-			}
-		}
-		if (stopKeyGetter != null)
-		{
-			stopPosition = (ExecIndexRow) stopKeyGetter.invoke(activation);
-		}
+        initStartAndStopKey();
 
 		// Check whether there are any comparisons with unordered nulls
 		// on either the start or stop position.  If there are, we can

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=816536&r1=816535&r2=816536&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
Fri Sep 18 08:46:29 2009
@@ -27,6 +27,7 @@
 import java.sql.Statement;
 import java.sql.SQLException;
 
+import java.sql.Types;
 import java.util.ArrayList;
 import java.util.BitSet;
 import java.util.Collections;
@@ -925,6 +926,22 @@
     }
 
     /**
+     * Statements with {@code X IN (?,?)} used to go into an infinite loop if
+     * the first parameter was NULL and there was an index on X (DERBY-4376).
+     */
+    public void testDerby4376() throws SQLException {
+        Statement s = createStatement();
+        s.execute("create table d4376(x int primary key)");
+        s.execute("insert into d4376 values (1), (2), (3)");
+        PreparedStatement ps = prepareStatement(
+                "select * from d4376 where x in (?, ?)");
+        ps.setNull(1, Types.INTEGER);
+        ps.setInt(2, 1);
+        JDBC.assertSingleValueResultSet(ps.executeQuery(), "1");
+        s.execute("drop table d4376");
+    }
+
+    /**
      * Insert the received number of rows into DATA_TABLE via
      * batch processing.
      */



Mime
View raw message