db-derby-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From banda...@apache.org
Subject svn commit: r345304 - in /db/derby/code/trunk/java: engine/org/apache/derby/iapi/reference/ engine/org/apache/derby/impl/sql/compile/ engine/org/apache/derby/loc/ testing/org/apache/derbyTesting/functionTests/master/ testing/org/apache/derbyTesting/fun...
Date Thu, 17 Nov 2005 18:55:39 GMT
Author: bandaram
Date: Thu Nov 17 10:55:30 2005
New Revision: 345304

URL: http://svn.apache.org/viewcvs?rev=345304&view=rev
Log:
DERBY-280: Prevent multiple columns from having same result column name in select-list, if
it has a group by/having clause also. Current Derby rewrite of these queries could lead to
wrong results.

Submitted by Rick Hillegas (Richard.Hillegas@Sun.COM)

Modified:
    db/derby/code/trunk/java/engine/org/apache/derby/iapi/reference/SQLState.java
    db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/sqlgrammar.jj
    db/derby/code/trunk/java/engine/org/apache/derby/loc/messages_en.properties
    db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/master/aggregate.out
    db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/aggregate.sql

Modified: db/derby/code/trunk/java/engine/org/apache/derby/iapi/reference/SQLState.java
URL: http://svn.apache.org/viewcvs/db/derby/code/trunk/java/engine/org/apache/derby/iapi/reference/SQLState.java?rev=345304&r1=345303&r2=345304&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/iapi/reference/SQLState.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/iapi/reference/SQLState.java Thu Nov
17 10:55:30 2005
@@ -1160,6 +1160,10 @@
 	String LANG_STOP_AFTER_BINDING                                     = "42Z56.U";
 	String LANG_STOP_AFTER_OPTIMIZING                                  = "42Z57.U";
 	String LANG_STOP_AFTER_GENERATING                                  = "42Z58.U";
+
+	// PARSER EXCEPTIONS
+	String LANG_UNBINDABLE_REWRITE                                     = "X0A00.S";
+	
 	// EXECUTION EXCEPTIONS
 	String LANG_CANT_LOCK_TABLE                                        = "X0X02.S";
 	String LANG_TABLE_NOT_FOUND_DURING_EXECUTION                       = "X0X05.S";

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/sqlgrammar.jj
URL: http://svn.apache.org/viewcvs/db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/sqlgrammar.jj?rev=345304&r1=345303&r2=345304&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/sqlgrammar.jj (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/sqlgrammar.jj Thu Nov
17 10:55:30 2005
@@ -148,6 +148,7 @@
 import org.apache.derby.iapi.util.StringUtil;
 
 import java.sql.Types;
+import java.util.Hashtable;
 import java.util.Properties;
 import java.util.Vector;
 import java.lang.Character;
@@ -310,6 +311,89 @@
 				version, feature);
 	}
 
+	/*
+	** Make sure that a select list in a parser-rewritten tree does
+	** not contain two columns by the same name. This is the patch
+	** to bug 280. In this case, the bind phase cannot tell the
+	** difference between the two columns and so the query may
+	** return wrong results.
+	**
+	** This issue is caused by parser rewriting of group by/having clauses
+	** into table expressions. (See function tableExpression() in this file)
+	** There is an improvement request filed under DERBY-681 to eliminate this
+	** rewrite, after which it should be possible to allow multiple columns to
+	** have same name in the select list.
+	*/
+	private	void	vetSelectList280( ResultColumnList selectList )
+		throws StandardException
+	{
+		Hashtable	allNames = new Hashtable();
+		int			count = selectList.size();
+
+		for ( int i = 0; i < count; i++ )
+		{
+			ResultColumn	rc = (ResultColumn) selectList.elementAt( i );
+			String			duplicateName = ambiguousDuplicateName( allNames, rc );
+
+			if ( duplicateName != null )
+			{
+				throw StandardException.newException
+				( SQLState.LANG_UNBINDABLE_REWRITE, duplicateName );
+			}
+
+		}
+	}
+
+	// allow two references to the same column. disallow aliasing
+	// two different expressions to the same name. returns the
+	// ambiguous column name if two result columns cannot be distinguished.
+	private	String	ambiguousDuplicateName( Hashtable allNames, ResultColumn rc )
+		throws StandardException
+	{
+		String			columnName = rc.getName();
+		ValueNode		newExpression = rc.getExpression();
+
+		// no column name means that the compiler will generate a
+		// unique name. that's ok and avoids bug 280
+		if ( columnName == null ) { return null; }
+
+		ValueNode		oldExpression = (ValueNode) allNames.get( columnName );
+
+		// no problem yet if we haven't seen this column
+		if ( oldExpression == null )
+		{
+			allNames.put( columnName, newExpression );
+			return null;
+		}
+
+		// if the two identically named references aren't both the
+		// same table column, then the bug occurs
+		if (
+				!(newExpression instanceof ColumnReference) ||
+				!(oldExpression instanceof ColumnReference)
+		   )
+		{ return columnName; }
+
+		ColumnReference		 newCR = (ColumnReference) newExpression;
+		ColumnReference		 oldCR = (ColumnReference) oldExpression;
+		TableName			 newTableName = newCR.getTableNameNode();
+		TableName			 oldTableName = oldCR.getTableNameNode();
+
+		if (
+				( (newTableName == null) && (oldTableName == null) ) ||
+				( (newTableName != null) && (newTableName.equals( oldTableName )) )
+		   )
+		{
+			// same table but different column aliased to same column name
+			if ( !newCR.getColumnName().equals( oldCR.getColumnName() ) )
+			{ return columnName; }
+			else { return null; }
+		}
+
+		// different tables
+		return null;
+	}
+
 	/**
 		Check that the current mode supports internal extensions.
 
@@ -7736,6 +7820,8 @@
 		 */
 		if (groupByList != null || havingClause != null)
 		{
+			vetSelectList280( selectList );
+
 			FromSubquery		fromSubquery;
 			ResultColumnList	outerRCL =
 										(ResultColumnList) nodeFactory.getNode(

Modified: db/derby/code/trunk/java/engine/org/apache/derby/loc/messages_en.properties
URL: http://svn.apache.org/viewcvs/db/derby/code/trunk/java/engine/org/apache/derby/loc/messages_en.properties?rev=345304&r1=345303&r2=345304&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/loc/messages_en.properties (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/loc/messages_en.properties Thu Nov 17
10:55:30 2005
@@ -907,6 +907,8 @@
 44X00.U=SQL Type Name
 44X05.U=next error
 
+X0A00.S=The select list mentions column ''{0}'' twice. This is not allowed in queries with
GROUP BY or HAVING clauses. Try aliasing one of the conflicting columns to a unique name.
+
 X0X02.S=Table ''{0}'' cannot be locked in ''{1}'' mode.
 X0X03.S=Invalid transaction state - held cursor requires same isolation level
 X0X05.S=Table ''{0}'' does not exist.

Modified: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/master/aggregate.out
URL: http://svn.apache.org/viewcvs/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/master/aggregate.out?rev=345304&r1=345303&r2=345304&view=diff
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/master/aggregate.out
(original)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/master/aggregate.out
Thu Nov 17 10:55:30 2005
@@ -889,4 +889,85 @@
 ERROR 42903: Invalid use of an aggregate function.
 ij> drop table tmax;
 0 rows inserted/updated/deleted
+ij> --
+-- JIRA Bug 280.
+--
+-- Giving two columns the same name confuses the wacky
+-- query rewriting which the parser undertakes for grouped
+-- and aggregated queries.
+--
+create table bug280
+(
+   a int,
+   b int
+);
+0 rows inserted/updated/deleted
+ij> insert into bug280( a, b )
+values ( 1, 1 ), ( 1, 2 ), ( 1, 3 ), ( 2, 1 ), ( 2, 2 );
+5 rows inserted/updated/deleted
+ij> --
+-- This is bug 280. The alias confused the grouping.
+-- The second query should return the same results
+-- as the first. However, this bug cannot be fixed
+-- until we clean up the query rewriting done by
+-- the parser (see bug 681). Until then, the second
+-- query will raise an exception advising the customer
+-- to rewrite the query.
+--
+select a, count( a )
+from bug280
+group by a;
+A          |2          
+-----------------------
+1          |3          
+2          |2          
+ij> -- should raise an error
+select a, count( a ) as a
+from bug280
+group by a;
+ERROR X0A00: The select list mentions column 'A' twice. This is not allowed in queries with
GROUP BY or HAVING clauses. Try aliasing one of the conflicting columns to a unique name.
+ij> -- should return same results as first query (but with extra column)
+select a, count( a ), a
+from bug280
+group by a;
+A          |2          |A          
+-----------------------------------
+1          |3          |1          
+2          |2          |2          
+ij> -- different tables with same column name ok
+select t.t_i, m.t_i from
+(select a, b from bug280 group by a, b) t (t_i, t_dt),
+(select a, b from bug280 group by a, b) m (t_i, t_dt)
+where t.t_i = m.t_i and t.t_dt = m.t_dt
+group by t.t_i, t.t_dt, m.t_i, m.t_dt order by t.t_i,m.t_i;
+T_I        |T_I        
+-----------------------
+1          |1          
+1          |1          
+1          |1          
+2          |2          
+2          |2          
+ij> -- should be allowed
+select a, a from bug280 group by a;
+A          |A          
+-----------------------
+2          |2          
+1          |1          
+ij> select bug280.a, a from bug280 group by a;
+A          |A          
+-----------------------
+2          |2          
+1          |1          
+ij> select bug280.a, bug280.a from bug280 group by a;
+A          |A          
+-----------------------
+2          |2          
+1          |1          
+ij> select a, bug280.a from bug280 group by a;
+A          |A          
+-----------------------
+2          |2          
+1          |1          
+ij> drop table bug280;
+0 rows inserted/updated/deleted
 ij> 

Modified: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/aggregate.sql
URL: http://svn.apache.org/viewcvs/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/aggregate.sql?rev=345304&r1=345303&r2=345304&view=diff
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/aggregate.sql
(original)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/aggregate.sql
Thu Nov 17 10:55:30 2005
@@ -403,3 +403,57 @@
 
 drop table tmax;
 
+--
+-- JIRA Bug 280.
+--
+-- Giving two columns the same name confuses the wacky
+-- query rewriting which the parser undertakes for grouped
+-- and aggregated queries.
+--
+
+create table bug280
+(
+   a int,
+   b int
+);
+
+insert into bug280( a, b )
+values ( 1, 1 ), ( 1, 2 ), ( 1, 3 ), ( 2, 1 ), ( 2, 2 );
+
+--
+-- This is bug 280. The alias confused the grouping.
+-- The second query should return the same results
+-- as the first. However, this bug cannot be fixed
+-- until we clean up the query rewriting done by
+-- the parser (see bug 681). Until then, the second
+-- query will raise an exception advising the customer
+-- to rewrite the query.
+--
+select a, count( a )
+from bug280
+group by a;
+
+-- should raise an error
+select a, count( a ) as a
+from bug280
+group by a;
+
+-- should return same results as first query (but with extra column)
+select a, count( a ), a
+from bug280
+group by a;
+
+-- different tables with same column name ok
+select t.t_i, m.t_i from
+(select a, b from bug280 group by a, b) t (t_i, t_dt),
+(select a, b from bug280 group by a, b) m (t_i, t_dt)
+where t.t_i = m.t_i and t.t_dt = m.t_dt
+group by t.t_i, t.t_dt, m.t_i, m.t_dt order by t.t_i,m.t_i;
+
+-- should be allowed
+select a, a from bug280 group by a;
+select bug280.a, a from bug280 group by a;
+select bug280.a, bug280.a from bug280 group by a;
+select a, bug280.a from bug280 group by a;
+
+drop table bug280;



Mime
View raw message