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 2016/03/13 00:19:32 UTC

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

Author: bpendleton
Date: Sat Mar 12 23:19:32 2016
New Revision: 1734744

URL: http://svn.apache.org/viewvc?rev=1734744&view=rev
Log:
DERBY-1773: cursor updates fail with syntax error when column has an alias

This change enhances the runtime column analysis code so that an updatable
cursor can make a more nuanced decision about whether a column update is
or is not allowed.

Specifically, certain columns may not be updated, if they have been aliased.

Prior to this change, a confusing syntax error message would be delivered
when attempting to update an aliased column. Now, a more clear error message
is delivered, pointing at the fact that the aliased column is not in the
FOR UPDATE list of the cursor.

So the net result is (at least, should be) that the same set of queries are
accepted, but those that are not accepted have a slightly more clear message
issued when they are detected.

Modified:
    db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/FromTable.java
    db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/ResultColumn.java
    db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/ResultColumnList.java
    db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/SelectNode.java
    db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/UpdatableResultSetTest.java

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/FromTable.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/FromTable.java?rev=1734744&r1=1734743&r2=1734744&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/FromTable.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/FromTable.java Sat Mar 12 23:19:32 2016
@@ -1438,6 +1438,19 @@ abstract class FromTable extends ResultS
 		getResultColumns().markUpdatableByCursor(updateColumns);
 	}
 
+ 	/**
+	 * Return true if some columns in this table are updatable.
+	 *
+	 * This method is used in deciding whether updateRow() or
+	 * insertRow() are allowable.
+	 *
+	 * @return true if some columns in this table are updatable.
+	 */
+	boolean columnsAreUpdatable()
+	{
+		return getResultColumns().columnsAreUpdatable();
+	}
+
 	/**
 	 * Flatten this FromTable into the outer query block. The steps in
 	 * flattening are:

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/ResultColumn.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/ResultColumn.java?rev=1734744&r1=1734743&r2=1734744&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/ResultColumn.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/ResultColumn.java Sat Mar 12 23:19:32 2016
@@ -359,6 +359,25 @@ class ResultColumn extends ValueNode
         }
 	}
 	
+ 	/**
+	 * Returns true if this column is updatable.
+	 *
+	 * This method is used for determining if updateRow and insertRow
+	 * are allowed for this cursor (DERBY-1773). Since the updateRow
+	 * and insertRow implementations dynamically build SQL statements
+	 * on the fly, the critical issue here is whether we have a
+	 * column that has been aliased, because if it has been
+	 * aliased, the dynamic SQL generation logic won't be able to
+	 * compute the proper true base column name when it needs to.
+	 *
+	 * @return true if this result column is updatable.
+	 */
+	boolean isUpdatable()
+	{
+		return _derivedColumnName == null ||
+			_underlyingName.equals(_derivedColumnName);
+	}
+
 	/**
 	 * Returns the underlying source column name, if this ResultColumn
 	 * is a simple direct reference to a table column, or NULL otherwise.

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/ResultColumnList.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/ResultColumnList.java?rev=1734744&r1=1734743&r2=1734744&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/ResultColumnList.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/ResultColumnList.java Sat Mar 12 23:19:32 2016
@@ -523,6 +523,21 @@ class ResultColumnList extends QueryTree
 		return retRC;
 	}
 
+ 	/**
+	 * Return true if some columns in this list are updatable.
+	 *
+	 * @return	true if any column in list is updatable, else false
+	 */
+	boolean columnsAreUpdatable()
+	{
+		for (ResultColumn rc : this)
+		{
+			if (rc.isUpdatable())
+				return true;
+		}
+		return false;
+	}
+		
 	/**
 	 * For order by column bind, get a ResultColumn that matches the specified 
 	 * columnName.
@@ -2692,10 +2707,14 @@ class ResultColumnList extends QueryTree
 	{
         for (ResultColumn rc : this)
 		{
-			//determine if the column is a base column and not a derived column
-            if (rc.getSourceTableName() != null) {
-                rc.markUpdatableByCursor();
-            }
+			// Determine whether the column is a base column and
+			// not a derived column, and, additionally,
+			// verify that the column was not aliased.
+			// 
+			if (rc.getSourceTableName() != null &&
+				rc.getExpression() != null &&
+				rc.getExpression().getColumnName().equals(rc.getName()))
+				rc.markUpdatableByCursor();
 		}
 	}
 	

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=1734744&r1=1734743&r2=1734744&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 Sat Mar 12 23:19:32 2016
@@ -2286,6 +2286,13 @@ class SelectNode extends ResultSetNode
 			return false;
 		}
 
+		if (! targetTable.columnsAreUpdatable())
+		{
+			if (SanityManager.DEBUG)
+				SanityManager.DEBUG("DumpUpdateCheck",
+				  "cursor select has no updatable result columns");
+			return false;
+		}
 
  		/* Get the TableDescriptor and verify that it is not for a
  		 * view or a system table.  

Modified: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/UpdatableResultSetTest.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/UpdatableResultSetTest.java?rev=1734744&r1=1734743&r2=1734744&view=diff
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/UpdatableResultSetTest.java (original)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/UpdatableResultSetTest.java Sat Mar 12 23:19:32 2016
@@ -1721,6 +1721,109 @@ public class UpdatableResultSetTest  ext
     }
     
     /**
+     * Tests for DERBY-1773, involving both explicit and implicit
+     * FOR UPDATE clauses, as well as various styles of specifying
+     * correlation names.
+     */
+    public void testUpdateRowWithTableAndColumnAlias_d1773()
+        throws SQLException
+    {
+        createTableT1();
+        Statement stmt = createStatement(
+            ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_UPDATABLE);
+
+        try {
+            // The presence of the alias for the columns should
+            // cause the statement to be rejected.
+            // 42Y90: FOR UPDATE is not permitted in this type of statement.
+            ResultSet rs = stmt.executeQuery(
+                "SELECT * from t1 as abcde(a1,a2) for update of c1");
+            fail("FAIL - executeQuery should have failed");
+        } catch (SQLException e) {
+            assertSQLState("42Y90", e);
+        }
+
+        try {
+            // The presence of the alias for the columns should
+            // cause the statement to be rejected.
+            ResultSet rs = stmt.executeQuery(
+                "SELECT * from t1 as abcde(a1,a2) for update");
+            fail("FAIL - executeQuery should have failed");
+        } catch (SQLException e) {
+            assertSQLState("42Y90", e);
+        }
+
+        // Without FOR UPDATE, not caught til execution time:
+        ResultSet rs = stmt.executeQuery(
+				"select * from t1 as a(a1,a2)");
+        rs.next();
+        try {
+            // Update should be rejected by correlation name
+            // on column 'updateString' not allowed because
+            // the ResultSet is not an updatable ResultSet.
+            rs.updateString(2, "bbbb");
+            fail("FAIL - updateString should have failed");
+        } catch (SQLException e) {
+            assertSQLState("XJ083", e);
+        }
+        rs.close();
+
+        rs = stmt.executeQuery(
+			"SELECT c1 as a1, c2 as a2 from t1 as abcde");
+        rs.next();
+        try {
+            // Update should be rejected by correlation name
+            // on column
+            //  Column 'A2' is not in the FOR UPDATE list of cursor 'SQLCUR0'.
+            rs.updateString(2, "bbbb");
+            fail("FAIL - updateString should have failed");
+        } catch (SQLException e) {
+            assertSQLState(usingDerbyNetClient() ? "XJ124" : "42X31", e);
+        }
+        rs.close();
+
+	// This update should probably work, but currently it is rejected.
+	// The idea is that only C1 should be read-only; c2 should be updatable
+	// 
+        rs = stmt.executeQuery(
+		"SELECT c1 as a1, c2 from t1 as abcde for update of c2");
+        rs.next();
+        // Update should be allowed on c2, not on c1.
+        rs.updateString(2, "bbbb");
+        try {
+            rs.updateString(1, "aaaa");
+            //  Column 'A1' is not in the FOR UPDATE list of cursor 'SQLCUR0'.
+            fail("FAIL - updateString should have failed");
+        } catch (SQLException e) {
+            assertSQLState(usingDerbyNetClient() ? "XJ124" : "42X31", e);
+        }
+        rs.close();
+
+	// Same as previous, but "for update" vs "for update of C2"
+        rs = stmt.executeQuery(
+		"SELECT c1 as a1, c2 from t1 as abcde for update");
+        rs.next();
+        // Update should be allowed on c2, not on c1.
+        rs.updateString(2, "bbbb");
+        try {
+            rs.updateString(1, "aaaa");
+            //  Column 'A1' is not in the FOR UPDATE list of cursor 'SQLCUR0'.
+            fail("FAIL - updateString should have failed");
+        } catch (SQLException e) {
+            assertSQLState(usingDerbyNetClient() ? "XJ124" : "42X31", e);
+        }
+        rs.close();
+
+        // No update should have occurred.
+        JDBC.assertFullResultSet(
+			stmt.executeQuery("SELECT * FROM t1"),
+            new String[][]{{"1", "aa"}, {"2", "bb"}, {"3", "cc"}},
+            true);
+
+        stmt.close();
+    }
+
+    /**
      * Positive test - 2 updatable resultsets going against the same table
      */
     public void testTwoResultSetsDeletingSameRow() throws SQLException {