db-derby-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From abr...@apache.org
Subject svn commit: r525925 - in /db/derby/code/trunk/java: engine/org/apache/derby/impl/sql/compile/HashJoinStrategy.java engine/org/apache/derby/impl/sql/compile/PredicateList.java testing/org/apache/derbyTesting/functionTests/tests/lang/DistinctTest.java
Date Thu, 05 Apr 2007 18:37:42 GMT
Author: abrown
Date: Thu Apr  5 11:37:40 2007
New Revision: 525925

URL: http://svn.apache.org/viewvc?view=rev&rev=525925
Log:
DERBY-2500: Fix PredicateList.orderUsefulPredicates() to recognize when we're
doing a hash join and to explicitly avoid generating probe predicates in that
case.  Also re-enable the "testResultSetInOrderWhenUsingIndex()" fixture for
lang/DistinctTest.java and add some additional test cases.  Finally, update
comments where appropriate to better explain the need for this restriction
on probe predicates.

Modified:
    db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/HashJoinStrategy.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/DistinctTest.java

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/HashJoinStrategy.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/HashJoinStrategy.java?view=diff&rev=525925&r1=525924&r2=525925
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/HashJoinStrategy.java
(original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/HashJoinStrategy.java
Thu Apr  5 11:37:40 2007
@@ -338,18 +338,19 @@
 							)
 						throws StandardException
 	{
-		/* If we're doing a Hash join then we shouldn't have any IN-list
-		 * probe predicates in the store restriction list.  The reason
-		 * is that those predicates are one-sided and thus if they
-		 * make it this far they will be pushed down to the base table
-		 * as restrictions on the rows read from disk.  That would be
-		 * wrong because a probe predicate is of the form "col = <val>"
-		 * where <val> is the first value in the IN-list.  But that's
-		 * not correct--we need to return all rows having any value that
-		 * appears in the IN-list (not just those rows matching the
-		 * first value).  Checks elsewhere in the code should ensure
-		 * that no probe predicates have made it this far, but if we're
-		 * running in SANE mode it doesn't hurt to verify.
+		/* We do not currently support IN-list "multi-probing" for hash scans
+		 * (though we could do so in the future).  So if we're doing a hash
+		 * join then we shouldn't have any IN-list probe predicates in the
+		 * store restriction list at this point.  The reason is that, in the
+		 * absence of proper multi-probing logic, such predicates would act
+		 * as restrictions on the rows read from disk.  That would be wrong
+		 * because a probe predicate is of the form "col = <val>" where <val>
+		 * is the first value in the IN-list.  Enforcement of that restriction
+		 * would lead to incorrect results--we need to return all rows having
+		 * any value that appears in the IN-list, not just those rows matching
+		 * the first value.  Checks elsewhere in the code should ensure that
+		 * no probe predicates have made it this far, but if we're running in
+		 * SANE mode it doesn't hurt to verify.
 		 */
 		if (SanityManager.DEBUG)
 		{

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?view=diff&rev=525925&r1=525924&r2=525925
==============================================================================
--- 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 Thu
Apr  5 11:37:40 2007
@@ -596,6 +596,35 @@
 		baseColumnPositions = cd.getIndexDescriptor().baseColumnPositions();
 		isAscending = cd.getIndexDescriptor().isAscending();
 
+		/* If we have a "useful" IN list probe predicate we will generate a
+		 * start/stop key for optTable of the form "col = <val>", where <val>
+		 * is the first value in the IN-list.  Then during normal index multi-
+		 * probing (esp. as implemented by exec/MultiProbeTableScanResultSet)
+		 * we will use that start/stop key as a "placeholder" into which we'll
+		 * plug the values from the IN-list one at a time.
+		 *
+		 * That said, if we're planning to do a hash join with optTable then
+		 * we won't generate a MultiProbeTableScanResult; instead we'll
+		 * generate a HashScanResultSet, which does not (yet) account for
+		 * IN-list multi-probing.  That means the start/stop key "col = <val>"
+		 * would be treated as a regular restriction, which could lead to
+		 * incorrect results.  So if we're dealing with a hash join, we do
+		 * not consider IN-list probe predicates to be "useful". DERBY-2500.
+		 *
+		 * Note that it should be possible to enhance HashScanResultSet to
+		 * correctly perform index multi-probing at some point, and there
+		 * would indeed be benefits to doing so (namely, we would scan fewer
+		 * rows from disk and build a smaller hash table). But until that
+		 * happens we have to make sure we do not consider probe predicates
+		 * to be "useful" for hash joins.
+		 *
+		 * Only need to do this check if "pushPreds" is true, i.e. if we're
+		 * modifying access paths and thus we know for sure that we are going
+		 * to generate a hash join.
+		 */
+		boolean skipProbePreds = pushPreds &&
+			optTable.getTrulyTheBestAccessPath().getJoinStrategy().isHashJoin();
+
 		/*
 		** Create an array of useful predicates.  Also, count how many
 		** useful predicates there are.
@@ -628,6 +657,14 @@
 			{
 				continue;
 			}
+
+			/* Skip it if we're doing a hash join and it's a probe predicate.
+			 * Then, since the probe predicate is deemed not useful, it will
+			 * be implicitly "reverted" to its underlying IN-list as part of
+			 * code generation.
+			 */
+			if (skipProbePreds && pred.isInListProbePredicate())
+				continue;
 
 			/* Look for an index column on one side of the relop */
 			for (indexPosition = 0;

Modified: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/DistinctTest.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/DistinctTest.java?view=diff&rev=525925&r1=525924&r2=525925
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/DistinctTest.java
(original)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/DistinctTest.java
Thu Apr  5 11:37:40 2007
@@ -469,10 +469,12 @@
 		s.close();
 	}
 	
-	/*
-	 * Following test case fails in the prepareStatement call,
-	 * with an ASSERT related to the DERBY-47 in list changes.
-	 *
+	/* Distinct query using ANDs and ORs, the latter of which will be
+	 * transformed into an IN list.  Assumption is that the optimizer
+	 * will choose to use an index for this query, though we don't
+	 * actually verify that (we're just checking that the query
+	 * compiles and executes without error).
+	 */
 	public void testResultSetInOrderWhenUsingIndex() throws SQLException{
 		Statement s = createStatement();
 		
@@ -494,28 +496,42 @@
 		s.execute("insert into netbuttonlibraryrole1 values('lusername2', 2,'user2', 'role2', default)");
 		
 		PreparedStatement p = prepareStatement("SELECT DISTINCT nb.name AS name, nb.summary AS
summary FROM netbutton1 nb, netbuttonlibraryrole1 nlr, library_netbutton ln" +
-		" WHERE nb.lname = ln.lname AND (nlr.lusername = ? OR nlr.lusername =?)");
-
-		p = prepareStatement("SELECT DISTINCT nb.name AS name, nb.summary AS summary FROM netbutton1
nb, netbuttonlibraryrole1 nlr, library_netbutton ln" +
 				" WHERE nlr.netbuttonlibrary_id = ln.netbuttonlibrary_id AND nb.lname = ln.lname AND
(nlr.lusername = ? OR nlr.lusername = ?) AND nb.lname = ? ORDER BY summary");
 		
 		p.setString(1, "lusername1");
 		p.setString(2, "lusername2");
-		//p.setString(3, "lname1");
+		p.setString(3, "lname1");
 		assertTrue(p.execute());
 
-	
 		String [][] expected = { {"name1", "sum2" } };
-    	ResultSet rs = p.getResultSet();
+		ResultSet rs = p.getResultSet();
 		JDBC.assertFullResultSet(rs, expected);
 		rs.close();
 		p.close();
 		
+		/* Similar to previous query but without the final equality predicate;
+		 * this query should return two rows.  Before the fix for DERBY-2500
+		 * we only returned one row, which was wrong.
+		 */
+		p = prepareStatement("SELECT DISTINCT nb.name AS name, nb.summary "
+			+ "AS summary FROM netbutton1 nb, netbuttonlibraryrole1 nlr, "
+			+ "library_netbutton ln WHERE nlr.netbuttonlibrary_id = "
+			+ "ln.netbuttonlibrary_id AND nb.lname = ln.lname AND "
+		 	+ "(nlr.lusername = ? OR nlr.lusername =?) ORDER BY summary");
+
+		p.setString(1, "lusername1");
+		p.setString(2, "lusername2");
+		assertTrue(p.execute());
+
+		expected = new String [][] { {"name1", "sum2" }, {"name2", "sum2"} };
+		rs = p.getResultSet();
+		JDBC.assertFullResultSet(rs, expected);
+		rs.close();
+
 		s.execute("drop table library_netbutton");
 		s.execute("drop table netbutton1");
 		s.close();
 	}
-	*/
 	
 	public void testDistinctStoreSort() throws SQLException {
 		Statement s = createStatement();



Mime
View raw message