db-derby-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kmars...@apache.org
Subject svn commit: r614017 - in /db/derby/code/trunk/java: engine/org/apache/derby/impl/sql/compile/ testing/org/apache/derbyTesting/functionTests/tests/lang/
Date Mon, 21 Jan 2008 21:29:53 GMT
Author: kmarsden
Date: Mon Jan 21 13:29:52 2008
New Revision: 614017

URL: http://svn.apache.org/viewvc?rev=614017&view=rev
Log:
DERBY-3257 SELECT with HAVING clause containing OR conditional incorrectly return 1 row -
should return 2 rows - works correctly with 10.2 DB

Normalize the havingClause before calling preprocess.  This made it necessary to explicitly
exclude having subqueries from flattenning, before they were implicitly excluded because the
clause was not normalized. Army Brown contributed the main part of the fix to normalize the
having clause.



Modified:
    db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/SelectNode.java
    db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/SubqueryList.java
    db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/SubqueryNode.java
    db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/GroupByTest.java

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/SelectNode.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/SelectNode.java?rev=614017&r1=614016&r2=614017&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/SelectNode.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/SelectNode.java Mon
Jan 21 13:29:52 2008
@@ -816,8 +816,11 @@
 		 * because the subquery transformations assume that any subquery operator 
 		 * negation has already occurred.
 		 */
-		normExpressions();
-
+		whereClause = normExpressions(whereClause);
+		// DERBY-3257. We need to normalize the having clause as well, because 
+		// preProcess expects CNF.
+		havingClause = normExpressions(havingClause);
+		
 		/**
 		 * This method determines if (1) the query is a LOJ, and (2) if the LOJ is a candidate
for
 		 * reordering (i.e., linearization).  The condition for LOJ linearization is:
@@ -880,7 +883,19 @@
 		}
 		
 		if (havingClause != null) {
-			havingClause = havingClause.preprocess(
+		    // DERBY-3257 
+		    // Mark  subqueries that are part of the having clause as 
+		    // such so we can avoid flattenning later. Having subqueries
+		    // cannot be flattened because we cannot currently handle
+		    // column references at the same source level.
+		    // DERBY-3257 required we normalize the having clause which
+		    // triggered flattening because SubqueryNode.underTopAndNode
+		    // became true after normalization.  We needed another way to
+		    // turn flattening off. Perhaps the long term solution is
+		    // to avoid this restriction all together but that was beyond
+		    // the scope of this bugfix.
+		    havingSubquerys.markHavingSubqueries();
+		    havingClause = havingClause.preprocess(
 					numTables, fromList, havingSubquerys, wherePredicates);
 		}
 		
@@ -1068,9 +1083,11 @@
 
 	/** Put the expression trees in conjunctive normal form 
 	 *
+     * @param boolClause clause to normalize
+     * 
 	 * @exception StandardException		Thrown on error
 	 */
-	private void normExpressions()
+	private ValueNode normExpressions(ValueNode boolClause)
 				throws StandardException
 	{
 		/* For each expression tree:
@@ -1079,41 +1096,43 @@
 		 *	  top level expression. (putAndsOnTop())
 		 *	o Finish the job (changeToCNF())
 		 */
-		if (whereClause != null)
+		if (boolClause != null)
 		{
-			whereClause = whereClause.eliminateNots(false);
+			boolClause = boolClause.eliminateNots(false);
 			if (SanityManager.DEBUG)
 			{
-				if (!(whereClause.verifyEliminateNots()) )
+				if (!(boolClause.verifyEliminateNots()) )
 				{
-					whereClause.treePrint();
+					boolClause.treePrint();
 					SanityManager.THROWASSERT(
-						"whereClause in invalid form: " + whereClause); 
+						"boolClause in invalid form: " + boolClause); 
 				}
 			}
-			whereClause = whereClause.putAndsOnTop();
+			boolClause = boolClause.putAndsOnTop();
 			if (SanityManager.DEBUG)
 			{
-				if (! ((whereClause instanceof AndNode) &&
-					   (whereClause.verifyPutAndsOnTop())) )
+				if (! ((boolClause instanceof AndNode) &&
+					   (boolClause.verifyPutAndsOnTop())) )
 				{
-					whereClause.treePrint();
+					boolClause.treePrint();
 					SanityManager.THROWASSERT(
-						"whereClause in invalid form: " + whereClause); 
+						"boolClause in invalid form: " + boolClause); 
 				}
 			}
-			whereClause = whereClause.changeToCNF(true);
+			boolClause = boolClause.changeToCNF(true);
 			if (SanityManager.DEBUG)
 			{
-				if (! ((whereClause instanceof AndNode) &&
-					   (whereClause.verifyChangeToCNF())) )
+				if (! ((boolClause instanceof AndNode) &&
+					   (boolClause.verifyChangeToCNF())) )
 				{
-					whereClause.treePrint();
+					boolClause.treePrint();
 					SanityManager.THROWASSERT(
-						"whereClause in invalid form: " + whereClause); 
+						"boolClause in invalid form: " + boolClause); 
 				}
 			}
 		}
+
+		return boolClause;
 	}
 
 	/**

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/SubqueryList.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/SubqueryList.java?rev=614017&r1=614016&r2=614017&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/SubqueryList.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/SubqueryList.java Mon
Jan 21 13:29:52 2008
@@ -237,5 +237,23 @@
 			((SubqueryNode) elementAt(index)).getResultSet().decrementLevel(decrement);
 		}
 	}
+
+	/**
+     * Mark all of the subqueries in this 
+     * list as being part of a having clause,
+     * so we can avoid flattenning later.
+	 * 
+	 */
+	public void markHavingSubqueries() {
+	    int size = size();
+	    
+	    for (int index = 0; index < size; index++)
+	    {
+	        SubqueryNode    subqueryNode;
+
+	        subqueryNode = (SubqueryNode) elementAt(index);
+	        subqueryNode.setHavingSubquery(true);
+	    }
+	}
 }
 

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/SubqueryNode.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/SubqueryNode.java?rev=614017&r1=614016&r2=614017&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/SubqueryNode.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/SubqueryNode.java Mon
Jan 21 13:29:52 2008
@@ -120,6 +120,12 @@
 	ValueNode		leftOperand;
 	boolean			pushedNewPredicate;
 
+    /**
+     * is this subquery part of a having clause.  We need to know this so 
+     * we can avoid flattening.
+     */
+    boolean havingSubquery = false;
+    
 	/* Expression subqueries on the right side of a BinaryComparisonOperatorNode
 	 * will get passed a pointer to that node prior to preprocess().  This
 	 * allows us to replace the entire comparison, if we want to, when
@@ -605,11 +611,12 @@
 
 		/* Values subquery is flattenable if:
 		 *  o It is not under an OR.
+         *  o It is not a subquery in a having clause (DERBY-3257)
 		 *  o It is an expression subquery on the right side
 		 *	  of a BinaryComparisonOperatorNode.
 		 */
 		flattenable = (resultSet instanceof RowResultSetNode) &&
-					  underTopAndNode &&
+					  underTopAndNode && !havingSubquery &&
 					  parentComparisonOperator instanceof BinaryComparisonOperatorNode;
 		if (flattenable)
 		{
@@ -666,6 +673,7 @@
 		 *  o There is a uniqueness condition that ensures
 		 *	  that the flattening of the subquery will not
 		 *	  introduce duplicates into the result set.
+         *  o The subquery is not part of a having clause (DERBY-3257)
 		 *
 		 *	OR,
 		 *  o The subquery is NOT EXISTS, NOT IN, ALL (beetle 5173).
@@ -673,7 +681,7 @@
 		boolean flattenableNotExists = (isNOT_EXISTS() || canAllBeFlattened());
 
 		flattenable = (resultSet instanceof SelectNode) &&
-					  underTopAndNode &&
+					  underTopAndNode && !havingSubquery &&
 					  (isIN() || isANY() || isEXISTS() || flattenableNotExists ||
                        parentComparisonOperator != null);
 
@@ -2284,5 +2292,22 @@
     protected boolean isEquivalent(ValueNode o)
     {
     	return false;
+    }
+
+    /**
+     * Is this subquery part of a having clause?
+     * 
+     * @return true if it is part of a having clause, otherwise false
+     */
+    public boolean isHavingSubquery() {
+        return havingSubquery;
+    }
+
+    /**
+     * Mark this subquery as being part of a having clause.
+     * @param havingSubquery
+     */
+    public void setHavingSubquery(boolean havingSubquery) {
+        this.havingSubquery = havingSubquery;
     }
 }

Modified: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/GroupByTest.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/GroupByTest.java?rev=614017&r1=614016&r2=614017&view=diff
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/GroupByTest.java
(original)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/GroupByTest.java
Mon Jan 21 13:29:52 2008
@@ -168,5 +168,26 @@
 
 		s.executeUpdate("drop table yy");
 	}
+    
+    
+    /**
+     * DERBY-3257 check for correct number of rows returned with
+     * or in having clause.
+     *  
+     * @throws SQLException
+     */
+    public void testOrNodeInHavingClause() throws SQLException
+    {
+        Statement s = createStatement();
+        s.executeUpdate("CREATE TABLE TAB ( ID VARCHAR(20), INFO VARCHAR(20))");
+        s.executeUpdate("insert into TAB values  ('1', 'A')");
+        s.executeUpdate("insert into TAB values  ('2', 'A')");
+        s.executeUpdate("insert into TAB values  ('3', 'B')");
+        s.executeUpdate("insert into TAB values  ('4', 'B')");
+        ResultSet rs = s.executeQuery("SELECT t0.INFO, COUNT(t0.ID) FROM TAB t0 GROUP BY
t0.INFO HAVING (t0.INFO = 'A' OR t0.INFO = 'B') AND t0.INFO IS NOT NULL");
+        String [][] expectedRows = {{"A","2"},{"B","2"}};
+        JDBC.assertFullResultSet(rs, expectedRows);
+        s.executeUpdate("DROP TABLE TAB");
+    }
 }
 



Mime
View raw message