Attached is a patch for current CVS, consisting of a cvs diff -c

for the changed files and a few new files:
- test/jdbc2/BatchExecuteTest.java
- util/MessageTranslator.java
- jdbc2/PBatchUpdateException.java

As an aside, is this the best way to submit a patch consisting
of both changed and new files? Or is there a smarter cvs command
which gets them all in one patch file?

This patch fixes batch processing in the JDBC driver to be
JDBC-2 compliant. Specifically, the changes introduced by this
patch are:

1) Statement.executeBatch() no longer commits or rolls back a
transaction, as this is not prescribed by the JDBC spec. Its up
to the application to disable autocommit and to commit or
rollback the transaction. Where JDBC talks about "executing the
statements as a unit", it means executing the statements in one
round trip to the backend for better performance, it does not
mean executing the statements in a transaction.

2) Statement.executeBatch() now throws a BatchUpdateException()
as required by the JDBC spec. The significance of this is that
the receiver of the exception gets the updateCounts of the
commands that succeeded before the error occurred. In order for
the messages to be translatable, java.sql.BatchUpdateException
is extended by org.postgresql.jdbc2.PBatchUpdateException() and
the localization code is factored out from
org.postgresql.util.PSQLException to a separate singleton class
org.postgresql.util.MessageTranslator.

3) When there is no batch or there are 0 statements in the batch
when Statement.executeBatch() is called, do not throw an
SQLException, but silently do nothing and return an update count
array of length 0. The JDBC spec says "Throws an SQLException if
the driver does not support batch statements", which is clearly
not the case. See testExecuteEmptyBatch() in
BatchExecuteTest.java for an example. The message
postgresql.stat.batch.empty is removed from the language
specific properties files.

4) When Statement.executeBatch() is performed, reset the
statement's list of batch commands to empty. The JDBC spec isn't
100% clear about this. This behaviour is only documented in the
Java tutorial
(http://java.sun.com/docs/books/tutorial/jdbc/jdbc2dot0/batchupdates.html).
Note that the Oracle JDBC driver also resets the statement's
list in executeBatch(), and this seems the most reasonable
interpretation.

5) A new test case is added to the JDBC test suite which tests
various aspects of batch processing. See the new file
BatchExecuteTest.java.

Regards,
Ren? Pijlman

6 files changed, 266 insertions, 43 deletions

diff --git a/src/interfaces/jdbc/org/postgresql/jdbc2/DatabaseMetaData.java b/src/interfaces/jdbc/org/postgresql/jdbc2/DatabaseMetaData.java
index 6acfec1421d..6e4c01c3331 100644
--- a/src/interfaces/jdbc/org/postgresql/jdbc2/DatabaseMetaData.java
+++ b/src/interfaces/jdbc/org/postgresql/jdbc2/DatabaseMetaData.java
@@ -2836,7 +2836,7 @@ public class DatabaseMetaData implements java.sql.DatabaseMetaData
     }
 
     /**
-     * New in 7.1 - If this is for PreparedStatement yes, ResultSet no
+     * Indicates whether the driver supports batch updates.
      */
     public boolean supportsBatchUpdates() throws SQLException
     {
diff --git a/src/interfaces/jdbc/org/postgresql/jdbc2/Statement.java b/src/interfaces/jdbc/org/postgresql/jdbc2/Statement.java
index b43454f6799..13dccefd39b 100644
--- a/src/interfaces/jdbc/org/postgresql/jdbc2/Statement.java
+++ b/src/interfaces/jdbc/org/postgresql/jdbc2/Statement.java
@@ -179,20 +179,26 @@ public class Statement extends org.postgresql.Statement implements java.sql.Stat
 
     public int[] executeBatch() throws SQLException
     {
-	if(batch==null || batch.isEmpty())
-	    throw new PSQLException("postgresql.stat.batch.empty");
-
+	if(batch==null)
+	    batch=new Vector();
 	int size=batch.size();
 	int[] result=new int[size];
 	int i=0;
-	this.execute("begin"); // PTM: check this when autoCommit is false
 	try {
 	    for(i=0;i<size;i++)
 		result[i]=this.executeUpdate((String)batch.elementAt(i));
-	    this.execute("commit"); // PTM: check this
 	} catch(SQLException e) {
-	    this.execute("abort"); // PTM: check this
-	    throw new PSQLException("postgresql.stat.batch.error",new Integer(i),batch.elementAt(i));
+		int[] resultSucceeded = new int[i];
+		System.arraycopy(result,0,resultSucceeded,0,i);
+
+		PBatchUpdateException updex =
+			new PBatchUpdateException("postgresql.stat.batch.error",
+			    new Integer(i), batch.elementAt(i), resultSucceeded);
+		updex.setNextException(e);
+
+		throw updex;
+	} finally {
+	    batch.removeAllElements();
 	}
 	return result;
     }
diff --git a/src/interfaces/jdbc/org/postgresql/test/JDBC2Tests.java b/src/interfaces/jdbc/org/postgresql/test/JDBC2Tests.java
index 96265dbe6d3..feeb0be5e9a 100644
--- a/src/interfaces/jdbc/org/postgresql/test/JDBC2Tests.java
+++ b/src/interfaces/jdbc/org/postgresql/test/JDBC2Tests.java
@@ -205,6 +205,10 @@ public class JDBC2Tests extends TestSuite {
     suite.addTestSuite(TimestampTest.class);
 
     // PreparedStatement
+    suite.addTestSuite(BatchExecuteTest.class);
+
+	// BatchExecute
+
 
     // MetaData
 
diff --git a/src/interfaces/jdbc/org/postgresql/test/jdbc2/BatchExecuteTest.java b/src/interfaces/jdbc/org/postgresql/test/jdbc2/BatchExecuteTest.java
new file mode 100644
index 00000000000..783bf7b67f0
--- /dev/null
+++ b/src/interfaces/jdbc/org/postgresql/test/jdbc2/BatchExecuteTest.java
@@ -0,0 +1,183 @@
+package org.postgresql.test.jdbc2;
+
+import org.postgresql.test.JDBC2Tests;
+import junit.framework.TestCase;
+import java.sql.*;
+
+/**
+ * Test case for Statement.batchExecute()
+ */
+public class BatchExecuteTest extends TestCase {
+
+	private Connection con;
+	private Statement stmt;
+
+	public BatchExecuteTest(String name) {
+		super(name);
+	}
+
+	// Set up the fixture for this testcase: a connection to a database with
+	// a table for this test.
+	protected void setUp() throws Exception {
+		con = JDBC2Tests.openDB();
+		stmt = con.createStatement();
+
+		// Drop the test table if it already exists for some reason. It is
+		// not an error if it doesn't exist.
+		try {
+			stmt.executeUpdate("DROP TABLE testbatch");
+		} catch (SQLException e) {
+			 // Intentionally ignore. We cannot distinguish "table does not
+			 // exist" from other errors, since PostgreSQL doesn't support
+			 // error codes yet.
+		}
+
+		stmt.executeUpdate("CREATE TABLE testbatch(pk INTEGER, col1 INTEGER)");
+		stmt.executeUpdate("INSERT INTO testbatch VALUES(1, 0)");
+
+		// Generally recommended with batch updates. By default we run all
+		// tests in this test case with autoCommit disabled.
+		con.setAutoCommit(false);
+	}
+
+	// Tear down the fixture for this test case.
+	protected void tearDown() throws Exception {
+		con.setAutoCommit(true);
+		if (stmt != null) {
+			stmt.executeUpdate("DROP TABLE testbatch");
+			stmt.close();
+		}
+		if (con != null) {
+			  JDBC2Tests.closeDB(con);
+		}
+	}
+
+	public void testSupportsBatchUpdates() throws Exception {
+		DatabaseMetaData dbmd = con.getMetaData();
+		assertTrue(dbmd.supportsBatchUpdates());
+	}
+
+	private void assertCol1HasValue(int expected) throws Exception {
+		Statement getCol1 = con.createStatement();
+
+		ResultSet rs =
+			getCol1.executeQuery("SELECT col1 FROM testbatch WHERE pk = 1");
+		assertTrue(rs.next());
+
+		int actual = rs.getInt("col1");
+
+		assertEquals(expected, actual);
+
+		assertEquals(false, rs.next());
+
+		rs.close();
+		getCol1.close();
+	}
+
+	public void testExecuteEmptyBatch() throws Exception {
+		int[] updateCount = stmt.executeBatch();
+		assertEquals(0,updateCount.length);
+
+		stmt.addBatch("UPDATE testbatch SET col1 = col1 + 1 WHERE pk = 1");
+		stmt.clearBatch();
+		updateCount = stmt.executeBatch();
+		assertEquals(0,updateCount.length);
+	}
+
+	public void testClearBatch() throws Exception {
+		stmt.addBatch("UPDATE testbatch SET col1 = col1 + 1 WHERE pk = 1");
+		assertCol1HasValue(0);
+		stmt.addBatch("UPDATE testbatch SET col1 = col1 + 2 WHERE pk = 1");
+		assertCol1HasValue(0);
+		stmt.clearBatch();
+		assertCol1HasValue(0);
+		stmt.addBatch("UPDATE testbatch SET col1 = col1 + 4 WHERE pk = 1");
+		assertCol1HasValue(0);
+		stmt.executeBatch();
+		assertCol1HasValue(4);
+		con.commit();
+		assertCol1HasValue(4);
+	}
+
+	public void testSelectThrowsException() throws Exception {
+		stmt.addBatch("UPDATE testbatch SET col1 = col1 + 1 WHERE pk = 1");
+		stmt.addBatch("SELECT col1 FROM testbatch WHERE pk = 1");
+		stmt.addBatch("UPDATE testbatch SET col1 = col1 + 2 WHERE pk = 1");
+
+		try {
+			stmt.executeBatch();
+			fail("Should raise a BatchUpdateException because of the SELECT");
+		} catch (BatchUpdateException e) {
+			int [] updateCounts = e.getUpdateCounts();
+			assertEquals(1,updateCounts.length);
+			assertEquals(1,updateCounts[0]);
+		} catch (SQLException e) {
+			fail( "Should throw a BatchUpdateException instead of " +
+			      "a generic SQLException: " + e);
+		}
+	}
+
+	public void testPreparedStatement() throws Exception {
+		PreparedStatement pstmt = con.prepareStatement(
+			"UPDATE testbatch SET col1 = col1 + ? WHERE PK = ?" );
+
+		// Note that the first parameter changes for every statement in the
+		// batch, whereas the second parameter remains constant.
+		pstmt.setInt(1,1);
+		pstmt.setInt(2,1);
+		pstmt.addBatch();
+		assertCol1HasValue(0);
+
+		pstmt.setInt(1,2);
+		pstmt.addBatch();
+		assertCol1HasValue(0);
+
+		pstmt.setInt(1,4);
+		pstmt.addBatch();
+		assertCol1HasValue(0);
+
+		pstmt.executeBatch();
+		assertCol1HasValue(7);
+
+		con.commit();
+		assertCol1HasValue(7);
+
+		con.rollback();
+		assertCol1HasValue(7);
+
+		pstmt.close();
+	}
+
+	/**
+	*/
+	public void testTransactionalBehaviour() throws Exception {
+		stmt.addBatch("UPDATE testbatch SET col1 = col1 + 1 WHERE pk = 1");
+		stmt.addBatch("UPDATE testbatch SET col1 = col1 + 2 WHERE pk = 1");
+		stmt.executeBatch();
+		con.rollback();
+		assertCol1HasValue(0);
+
+		stmt.addBatch("UPDATE testbatch SET col1 = col1 + 4 WHERE pk = 1");
+		stmt.addBatch("UPDATE testbatch SET col1 = col1 + 8 WHERE pk = 1");
+
+		// The statement has been added to the batch, but it should not yet
+		// have been executed.
+		assertCol1HasValue(0);
+
+		int[] updateCounts = stmt.executeBatch();
+		assertEquals(2,updateCounts.length);
+		assertEquals(1,updateCounts[0]);
+		assertEquals(1,updateCounts[1]);
+
+		assertCol1HasValue(12);
+		con.commit();
+		assertCol1HasValue(12);
+		con.rollback();
+		assertCol1HasValue(12);
+	}
+}
+
+/* TODO tests that can be added to this test case
+   - SQLExceptions chained to a BatchUpdateException
+   - test PreparedStatement as thoroughly as Statement
+ */
diff --git a/src/interfaces/jdbc/org/postgresql/util/MessageTranslator.java b/src/interfaces/jdbc/org/postgresql/util/MessageTranslator.java
new file mode 100644
index 00000000000..97fd32a8696
--- /dev/null
+++ b/src/interfaces/jdbc/org/postgresql/util/MessageTranslator.java
@@ -0,0 +1,63 @@
+package org.postgresql.util;
+
+import java.util.*;
+import java.text.*;
+
+/**
+ * A singleton class to translate JDBC driver messages in SQLException's.
+ */
+public class MessageTranslator {
+
+	// The singleton instance.
+	private static MessageTranslator instance = null;
+
+	private ResourceBundle bundle;
+
+	private MessageTranslator() {
+		try {
+			bundle = ResourceBundle.getBundle("org.postgresql.errors");
+	    } catch(MissingResourceException e) {
+			// translation files have not been installed.
+			bundle = null;
+	    }
+	}
+
+	// Synchronized, otherwise multiple threads may perform the test and
+	// assign to the singleton instance simultaneously.
+    private synchronized final static MessageTranslator getInstance() {
+		if (instance == null) {
+			instance = new MessageTranslator();
+		}
+		return instance;
+	}
+
+	public final static String translate(String id, Object[] args) {
+
+		MessageTranslator translator = MessageTranslator.getInstance();
+
+		return translator._translate(id, args);
+	}
+
+	private final String _translate(String id, Object[] args) {
+		String message;
+
+        if (bundle != null && id != null) {
+			// Now look up a localized message. If one is not found, then use
+			// the supplied message instead.
+			try {
+				message = bundle.getString(id);
+			} catch(MissingResourceException e) {
+				message = id;
+			}
+        } else {
+			message = id;
+		}
+
+		// Expand any arguments
+		if (args != null && message != null) {
+			message = MessageFormat.format(message,args);
+		}
+
+		return message;
+    }
+}
diff --git a/src/interfaces/jdbc/org/postgresql/util/PSQLException.java b/src/interfaces/jdbc/org/postgresql/util/PSQLException.java
index 932bf6e3578..d5c8cefa7df 100644
--- a/src/interfaces/jdbc/org/postgresql/util/PSQLException.java
+++ b/src/interfaces/jdbc/org/postgresql/util/PSQLException.java
@@ -2,8 +2,6 @@ package org.postgresql.util;
 
 import java.io.*;
 import java.sql.*;
-import java.text.*;
-import java.util.*;
 
 /**
  * This class extends SQLException, and provides our internationalisation handling
@@ -12,9 +10,6 @@ public class PSQLException extends SQLException
 {
     private String message;
 
-    // Cache for future errors
-    static ResourceBundle bundle;
-
     /**
      * This provides the same functionality to SQLException
      * @param error Error string
@@ -86,37 +81,10 @@ public class PSQLException extends SQLException
 	translate(error,argv);
     }
 
-    /**
-     * This does the actual translation
-     */
-    private void translate(String id,Object[] args)
-    {
-	if(bundle == null) {
-	    try {
-		bundle = ResourceBundle.getBundle("org.postgresql.errors");
-	    } catch(MissingResourceException e) {
-                // translation files have not been installed.
-                message = id;
-	    }
+	private void translate(String error, Object[] args) {
+		message = MessageTranslator.translate(error,args);
 	}
 
-        if (bundle != null) {
-	// Now look up a localized message. If one is not found, then use
-	// the supplied message instead.
-	    message = null;
-	    try {
-	        message = bundle.getString(id);
-	    } catch(MissingResourceException e) {
-	        message = id;
-	    }
-        }
-
-	// Expand any arguments
-	if(args!=null && message != null)
-	    message = MessageFormat.format(message,args);
-
-    }
-
     /**
      * Overides Throwable
      */
@@ -140,5 +108,4 @@ public class PSQLException extends SQLException
     {
 	return message;
     }
-
 }