Author: dag
Date: Sun Mar 15 01:30:42 2009
New Revision: 754579

URL: http://svn.apache.org/viewvc?rev=754579&view=rev
Log:
DERBY-4071 AssertFailure when selecting rows from a table with CHARACTER and VARCHAR columns

Patch derby-4071 which fixes this issue.

Modified:
    db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/GroupByNode.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/GroupByNode.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/GroupByNode.java?rev=754579&r1=754578&r2=754579&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/GroupByNode.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/GroupByNode.java Sun Mar 15 01:30:42 2009
@@ -336,12 +336,17 @@
 	}
 
 	/**
-	 * In the query rewrite for group by, add the columns on which
-	 * we are doing the group by.
-
+	 * In the query rewrite for group by, add the columns on which we are doing
+	 * the group by.
+	 *
+	 * @return havingRefsToSubstitute visitors array. Return any
+	 *         havingRefsToSubstitute visitors since it is too early to apply
+	 *         them yet; we need the AggregateNodes unmodified until after
+	 *         we add the new columns for aggregation (DERBY-4071).
+	 *
 	 * @see #addNewColumnsForAggregation
 	 */
-	private void addUnAggColumns() throws StandardException
+	private ArrayList addUnAggColumns() throws StandardException
 	{
 		ResultColumnList bottomRCL  = childResult.getResultColumns();
 		ResultColumnList groupByRCL = resultColumns;
@@ -452,10 +457,13 @@
 		if (havingRefsToSubstitute != null)
 		{
 			Collections.sort(havingRefsToSubstitute,sorter);
-			for (int r = 0; r < havingRefsToSubstitute.size(); r++)
-				havingClause.accept(
-					(SubstituteExpressionVisitor)havingRefsToSubstitute.get(r));
-}
+			// DERBY-4071 Don't substitute quite yet; we need the AggrateNodes
+			// undisturbed until after we have had the chance to build the
+			// other columns.  (The AggrateNodes are shared via an alias from
+			// aggregateVector and from the expression tree under
+			// havingClause).
+		}
+		return havingRefsToSubstitute;
 	}
 
 	/**
@@ -535,11 +543,26 @@
 		throws StandardException
 	{
 		aggInfo = new AggregatorInfoList();
+		ArrayList havingRefsToSubstitute = null;
+
 		if (groupingList != null)
 		{
-			addUnAggColumns();
+			havingRefsToSubstitute = addUnAggColumns();
 		}
+
+		addAggregateColumns();
+
 		if (havingClause != null) {
+
+			// Now do the substitution of the group by expressions in the
+			// having clause.
+			if (havingRefsToSubstitute != null) {
+				for (int r = 0; r < havingRefsToSubstitute.size(); r++) {
+					havingClause.accept(
+						(SubstituteExpressionVisitor)havingRefsToSubstitute.get(r));
+				}
+			}
+
 			// we have replaced group by expressions in the having clause.
 			// there should be no column references in the having clause 
 			// referencing this table. Skip over aggregate nodes.
@@ -565,7 +588,7 @@
 				}
 			}
 		}
-		addAggregateColumns();
+
 	}
 	
 	/**
@@ -650,7 +673,7 @@
 			** Set the GB aggregrate result column to
 			** point to this.  The GB aggregate result
 			** was created when we called
-			** ReplaceAggregatesWithColumnReferencesVisitor()
+			** ReplaceAggregatesWithCRVisitor()
 			*/
 			newColumnRef = (ColumnReference) getNodeFactory().getNode(
 					C_NodeTypes.COLUMN_REFERENCE,

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=754579&r1=754578&r2=754579&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 Sun Mar 15 01:30:42 2009
@@ -125,6 +125,15 @@
         st.executeUpdate("insert into d2457_a values (1, 12), (2, 23), " +
                 "(3, 34), (4, 45), (5, 56)");
 
+        st.executeUpdate("create table d4071(i int, v char(10))");
+        st.executeUpdate("insert into d4071 " +
+                         "       values (1, '0123456789')," +
+                         "              (1, '1234567890')," +
+                         "              (3, '2345678901')," +
+                         "              (4, '0123456789')," +
+                         "              (5, '1234567890')");
+
+
         // create an all types tables
         
         st.executeUpdate(
@@ -2218,4 +2227,20 @@
          rs = s.executeQuery("SELECT GroupCol, MAXOF2(SUM(Value1), SUM(Value2)) AS MaxOf2 FROM Testd3631 GROUP BY GroupCol");
          JDBC.assertFullResultSet(rs, new String[][] {{"1","5.0"},{"2","-3.0"}});
     }
+
+
+    /**
+     * Test aggregate function on a GROUP BY column also present in a HAVING
+     * clause.  Iff the GROUP BY column is not the first column in the table,
+     * this would fail before DERBY-4071 was fixed.
+     *
+     * @throws SQLException
+     */
+    public void testDerby4071AggregateOnGroupByColumnInHaving() throws SQLException {
+         Statement s = createStatement();
+         ResultSet rs = s.executeQuery("SELECT MAX(i), COUNT(T.V) FROM d4071 T " +
+                                       "    GROUP BY T.V HAVING COUNT(T.V) > 1");
+         
+         JDBC.assertFullResultSet(rs, new String[][] {{"4","2"},{"5","2"}});
+    }
 }
\ No newline at end of file