You are viewing a plain text version of this content. The canonical link for it is here.
Posted to derby-commits@db.apache.org by bp...@apache.org on 2006/11/05 17:45:25 UTC

svn commit: r471459 - in /db/derby/code/trunk/java: engine/org/apache/derby/impl/sql/compile/ testing/org/apache/derbyTesting/functionTests/master/ testing/org/apache/derbyTesting/functionTests/tests/lang/

Author: bpendleton
Date: Sun Nov  5 08:45:24 2006
New Revision: 471459

URL: http://svn.apache.org/viewvc?view=rev&rev=471459
Log:
DERBY-2014: NullPointerException with NULLIF in GROUP BY clause

This change was contributed by Yip Ng (yipng168@gmail.com)

The NPE happens in isEquivalent() method where it does not handle
value is null. (same symptom as DERBY-2008) and the patch addresses
this + additonal testcases.

The isEquivalent() method is used to compare the select column
against the group by with expression. Note that it is comparing
the structural form of the two expressions for equivalence at bind phase
and not comparing the actual row values at runtime to produce a result.

This patch converts all the tests in the previous patch into junit.
Also the javadoc for ValueNode.isEquivalent() method has been updated.

Modified:
    db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/ConstantNode.java
    db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/ValueNode.java
    db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/master/groupBy.out
    db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/GroupByExpressionTest.java
    db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/groupBy.sql

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/ConstantNode.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/ConstantNode.java?view=diff&rev=471459&r1=471458&r2=471459
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/ConstantNode.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/ConstantNode.java Sun Nov  5 08:45:24 2006
@@ -282,7 +282,11 @@
 	{
 		if (isSameNodeType(o)) {
 			ConstantNode other = (ConstantNode)o;
-			return other.getValue().compare(getValue()) == 0;
+			
+			// value can be null which represents a SQL NULL value.
+			return ( (other.getValue() == null && getValue() == null) || 
+					 (other.getValue() != null && 
+							 other.getValue().compare(getValue()) == 0) );
 		}
 		return false;
 	}

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/ValueNode.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/ValueNode.java?view=diff&rev=471459&r1=471458&r2=471459
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/ValueNode.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/ValueNode.java Sun Nov  5 08:45:24 2006
@@ -1334,7 +1334,45 @@
 	 * Tests if this node is equivalent to the specified ValueNode. Two 
 	 * ValueNodes are considered equivalent if they will evaluate to the same
 	 * value during query execution. 
+	 * <p> 
+	 * This method provides basic expression matching facility for the derived 
+	 * class of ValueNode and it is used by the language layer to compare the 
+	 * node structural form of the two expressions for equivalence at bind 
+	 * phase.  
+	 *  <p>
+	 * Note that it is not comparing the actual row values at runtime to produce 
+	 * a result; hence, when comparing SQL NULLs, they are considered to be 
+	 * equivalent and not unknown.  
+	 *  <p>
+	 * One usage case of this method in this context is to compare the select 
+	 * column expression against the group by expression to check if they are 
+	 * equivalent.  e.g.:
+	 *  <p>
+	 * SELECT c1+c2 FROM t1 GROUP BY c1+c2   
+	 *  <p>
+	 * In general, node equivalence is determined by the derived class of 
+	 * ValueNode.  But they generally abide to the rules below:
+	 *  <ul>
+	 * <li>The two ValueNodes must be of the same node type to be considered 
+	 *   equivalent.  e.g.:  CastNode vs. CastNode - equivalent (if their args 
+	 *   also match), ColumnReference vs CastNode - not equivalent.
+	 *   
+	 * <li>If node P contains other ValueNode(s) and so on, those node(s) must 
+	 *   also be of the same node type to be considered equivalent.
+	 *   
+	 * <li>If node P takes a parameter list, then the number of arguments and its 
+	 *   arguments for the two nodes must also match to be considered 
+	 *   equivalent.  e.g.:  CAST(c1 as INTEGER) vs CAST(c1 as SMALLINT), they 
+	 *   are not equivalent.
+	 *   
+	 * <li>When comparing SQL NULLs in this context, they are considered to be 
+	 *   equivalent.
 	 * 
+	 * <li>If this does not apply or it is determined that the two nodes are not 
+	 *   equivalent then the derived class of this method should return false; 
+	 *   otherwise, return true.
+	 * </ul>   
+	 *   
 	 * @param other the node to compare this ValueNode against.
 	 * @return <code>true</code> if the two nodes are equivalent, 
 	 * <code>false</code> otherwise.

Modified: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/master/groupBy.out
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/master/groupBy.out?view=diff&rev=471459&r1=471458&r2=471459
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/master/groupBy.out (original)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/master/groupBy.out Sun Nov  5 08:45:24 2006
@@ -636,28 +636,4 @@
 10         |2          
 ij> drop table t;
 0 rows inserted/updated/deleted
-ij> -- DERBY-2008
--- test SUBSTR with 2 args with GROUP BY expression
-create table dt (vc varchar(30));
-0 rows inserted/updated/deleted
-ij> insert into dt values ('1928-09-21'), ('1903-12-08');
-2 rows inserted/updated/deleted
-ij> -- ok
-select substr(vc, 3) from dt group by substr(vc, 3);
-1                             
-------------------------------
-03-12-08                      
-28-09-21                      
-ij> select substr(vc, 3, 4) from dt group by substr(vc, 3, 4);
-1   
-----
-03-1
-28-0
-ij> -- expect errors
-select substr(vc, 3, 4) from dt group by substr(vc, 3);
-ERROR 42Y30: The SELECT list of a grouped query contains at least one invalid expression. If a SELECT list has a GROUP BY, the list may only contain valid grouping expressions and valid aggregate expressions.  
-ij> select substr(vc, 3) from dt group by substr(vc, 3, 4);
-ERROR 42Y30: The SELECT list of a grouped query contains at least one invalid expression. If a SELECT list has a GROUP BY, the list may only contain valid grouping expressions and valid aggregate expressions.  
-ij> drop table dt;
-0 rows inserted/updated/deleted
 ij> 

Modified: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/GroupByExpressionTest.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/GroupByExpressionTest.java?view=diff&rev=471459&r1=471458&r2=471459
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/GroupByExpressionTest.java (original)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/GroupByExpressionTest.java Sun Nov  5 08:45:24 2006
@@ -36,7 +36,7 @@
 public class GroupByExpressionTest extends BaseJDBCTestCase
 {
 
-    private static String[][] TABLES = { 
+	private static String[][] TABLES = { 
         {"test", "create table test (c1 int, c2 int, c3 int, c4 int)"},
         {"coal", "create table coal (vc1 varchar(2), vc2 varchar(2))"},
         {"alltypes", 
@@ -45,7 +45,12 @@
             " d double precision, r real, " + 
             " dt date, t time, ts timestamp, " +
             " b char(2) for bit data, bv varchar(8) for bit data, " +
-            " lbv long varchar for bit data, dc decimal(5,2))"}};
+            " lbv long varchar for bit data, dc decimal(5,2))"},
+        {"t1", "create table t1 (c1 varchar(30))"},
+        {"t2", "create table t2 (c1 varchar(10))"},
+        {"t3", "create table t3 (c1 int, c2 int)"}
+    };
+
     private static String[][] FUNCTIONS = {
         {"r", "create function r() returns double external name " +
             "'java.lang.Math.random' language java parameter style java"}};
@@ -182,6 +187,81 @@
                         {"dupl", new Integer(14)},
                         {"good", new Integer(1)}});
 
+        // DERBY-2008 
+        // substr (2-args)
+        verifyQueryResults(
+                "substr-Q1",
+                "select substr(c1, 3) from t1 group by substr(c1, 3)",
+                new String[][] { {"03-12-08"},
+                                 {"28-09-21"} });
+        // substr (3-args)
+        verifyQueryResults(
+                "substr-Q2",
+                "select substr(c1, 3, 4) from t1 group by substr(c1, 3, 4)",
+                new String[][] { {"03-1"},
+                                 {"28-0"} });
+
+        // ltrim
+        verifyQueryResults(
+                "ltrim",
+                "select ltrim(c1) from t2 group by ltrim(c1)",
+                new String[][] { {"123 "},
+                                 {"abc "} });
+
+        // rtrim
+        verifyQueryResults(
+                "rtrim",
+                "select rtrim(c1) from t2 group by rtrim(c1)",
+                new String[][] { {"123"},
+                                 {"abc"} });
+
+        // locate (2-args)
+        verifyQueryResults(
+                "locate-Q1",
+                "select locate(c1, 'abc') from t2 group by locate(c1, 'abc')",
+                new int[][] { { 0 }, 
+                              { 1 } });
+
+        // locate (3-args)
+        verifyQueryResults(
+                "locate-Q2",
+                "select locate(c1, 'abc', 1) from t2 group by locate(c1, 'abc',1)",
+                new int[][] { { 0 }, 
+                              { 1 } });
+        
+        // cast with NULL
+        verifyQueryResults(
+                "cast-Q2",
+                "select (cast (NULL as INTEGER)) from t2 group by (cast (NULL as INTEGER))",
+                new Object[][] { { null } } );
+
+        // DERBY-2014
+        // nullif
+        verifyQueryResults(
+                "nullif-Q1",
+                "select nullif(c1,c1) from t3 group by nullif(c1,c1)",
+                new Object[][] { { null } } );
+
+        verifyQueryResults(
+                "nullif-Q2",
+                "select nullif(c1,c2) from t3 group by nullif(c1,c2)",
+                new Object[][] { { new Integer(5) }, 
+                                 { null } });
+
+        verifyQueryResults(
+                "nullif-Q3",
+                "select nullif(c1,10) from t3 group by nullif(c1,10)",
+                new Object[][] { { new Integer(1) },
+                                 { new Integer(2) },
+                                 { new Integer(3) },
+                                 { new Integer(5) },
+                                 { null } });
+
+        verifyQueryResults(
+                "nullif-Q4",
+                "select nullif(1,c1) from t3 group by nullif(1,c1)",
+                new Object[][] { { new Integer(1) }, 
+                                 { null } });
     }
     
     public void testExtractOperator() throws Exception
@@ -263,6 +343,34 @@
         assertCompileError(
                 "42Y30",
                 "select substr(c, 3, 4) from alltypes group by substr(v, 3, 4)");
+
+        // DERBY-2008
+        // invalid grouping expression 
+        assertCompileError(
+                "42Y30",
+                "select substr(c1, 3, 4) from t1 group by substr(c1, 3)");
+        assertCompileError(
+                "42Y30",
+                "select substr(c1, 3) from t1 group by substr(c1, 3, 4)");
+        assertCompileError(
+                "42Y30",
+                "select locate(c1, 'abc') from t2 group by locate(c1, 'abc',3)");
+        assertCompileError(
+                "42Y30",
+                "select locate(c1, 'abc',2) from t2 group by locate(c1, 'abc')");
+        assertCompileError(
+                "42Y30",
+                "select locate(c1, 'abc',2) from t2 group by locate(c1, 'abc',3)");
+
+        // DERBY-2014
+        // invalid grouping expression
+        assertCompileError(
+                "42Y30",
+                "select nullif(c1,c2) from t3 group by nullif(c2,c1)");
+        assertCompileError(
+                "42Y30",
+                "select nullif(c1,100) from t3 group by nullif(c1,200)");
+
         // aggregates in group by list.
         assertCompileError(
                 "42Y26",
@@ -463,7 +571,11 @@
                     " date('1992-03-04'), time('12:30:42'), " + 
                     " timestamp('1992-03-04 12:30:42'), " +
                     " X'12af', X'1111111111111111', X'1234', 111.11) " );
-                
+
+                s.execute("insert into t1 values ('1928-09-21'), ('1903-12-08')");
+                s.execute("insert into t2 values '123 ', 'abc ', '123', 'abc'") ;
+                s.execute("insert into t3 values (1,1), (2,2), (2,2), (3,3), (null, null), (5,100)");
+
                 s.close();
                 c.commit();
                 c.close();

Modified: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/groupBy.sql
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/groupBy.sql?view=diff&rev=471459&r1=471458&r2=471459
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/groupBy.sql (original)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/groupBy.sql Sun Nov  5 08:45:24 2006
@@ -360,15 +360,3 @@
 -- ok, gives one row
 select 10,avg(c) from t having 1 < 2;
 drop table t;
-
--- DERBY-2008
--- test SUBSTR with 2 args with GROUP BY expression
-create table dt (vc varchar(30));
-insert into dt values ('1928-09-21'), ('1903-12-08');
--- ok
-select substr(vc, 3) from dt group by substr(vc, 3); 
-select substr(vc, 3, 4) from dt group by substr(vc, 3, 4); 
--- expect errors
-select substr(vc, 3, 4) from dt group by substr(vc, 3); 
-select substr(vc, 3) from dt group by substr(vc, 3, 4);
-drop table dt;