Re: [PATCHES] JDBC Statement.executeBatch patch

Rene,

The patch looks good. Thanks for your time and effort.

thanks,
--Barry

Rene Pijlman wrote:
> 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
>
>
> ------------------------------------------------------------------------
>
> Index: src/interfaces/jdbc/org/postgresql/errors.properties
> ===================================================================
> RCS file: /home/projects/pgsql/cvsroot/pgsql/src/interfaces/jdbc/org/postgresql/errors.properties,v
> retrieving revision 1.6
> diff -c -r1.6 errors.properties
> *** src/interfaces/jdbc/org/postgresql/errors.properties 2001/08/21 00:37:23 1.6
> --- src/interfaces/jdbc/org/postgresql/errors.properties 2001/08/26 18:20:15
> ***************
> *** 61,67 ****
> postgresql.serial.noclass:No class found for {0}.
> postgresql.serial.table:The table for {0} is not in the database. Contact the DBA, as the database is in an inconsistent state.
> postgresql.serial.underscore:Class names may not have _ in them. You supplied {0}.
> - postgresql.stat.batch.empty:The batch is empty. There is nothing to execute.
> postgresql.stat.batch.error:Batch entry {0} {1} was aborted.
> postgresql.stat.maxfieldsize:An attempt to setMaxFieldSize() failed - compile time default in force.
> postgresql.stat.noresult:No results were returned by the query.
> --- 61,66 ----
> Index: src/interfaces/jdbc/org/postgresql/errors_de.properties
> ===================================================================
> RCS file: /home/projects/pgsql/cvsroot/pgsql/src/interfaces/jdbc/org/postgresql/errors_de.properties,v
> retrieving revision 1.1
> diff -c -r1.1 errors_de.properties
> *** src/interfaces/jdbc/org/postgresql/errors_de.properties 2001/07/09 20:25:44 1.1
> --- src/interfaces/jdbc/org/postgresql/errors_de.properties 2001/08/26 18:20:15
> ***************
> *** 65,71 ****
> postgresql.serial.noclass:Keine Klasse für Typ »{0}« gefunden
> postgresql.serial.table:Keine Tabelle für Typ »{0}« in der Datenbank gefunden. Die Datenbank ist in einem unbeständigen Zustand.
> postgresql.serial.underscore:Zu serialisierende Klassennamen dürfen keine Unterstriche (_) enthälten. Der angegebene Name war »{0}«.
> - postgresql.stat.batch.empty:Der Anweisungs-Batch ist leer.
> postgresql.stat.batch.error:Batch-Anweisung Nummer {0} ({1}) wurde abgebrochen.
> postgresql.stat.maxfieldsize:setMaxFieldSize() is nicht möglich; die Grenze ist fest eingebaut.
> postgresql.stat.noresult:Die Abfrage ergab kein Ergebnis.
> --- 65,70 ----
> Index: src/interfaces/jdbc/org/postgresql/errors_it.properties
> ===================================================================
> RCS file: /home/projects/pgsql/cvsroot/pgsql/src/interfaces/jdbc/org/postgresql/errors_it.properties,v
> retrieving revision 1.1
> diff -c -r1.1 errors_it.properties
> *** src/interfaces/jdbc/org/postgresql/errors_it.properties 2000/10/12 08:55:24 1.1
> --- src/interfaces/jdbc/org/postgresql/errors_it.properties 2001/08/26 18:20:15
> ***************
> *** 60,66 ****
> postgresql.serial.noclass:Nessuna classe trovata per {0}.
> postgresql.serial.table:La tabella per {0} non è nel database. Contattare il DBA, visto che il database è in uno stato incosistente.
> postgresql.serial.underscore:Il nome di una classe non può contenere il carattere ``_''. E` stato fornito {0}.
> - postgresql.stat.batch.empty:La sequenza di operazioni è vuota. Non c'è niente da eseguire.
> postgresql.stat.batch.error:L'operazione {0} {1} della sequenza è stata annullata.
> postgresql.stat.maxfieldsize:Fallito un tentativo a setMaxFieldSize() - verrà utilizzato il valore predefinito a tempo di compilazione.
> postgresql.stat.noresult:Nessun risultato è stato restituito dalla query.
> --- 60,65 ----
> Index: src/interfaces/jdbc/org/postgresql/errors_nl.properties
> ===================================================================
> RCS file: /home/projects/pgsql/cvsroot/pgsql/src/interfaces/jdbc/org/postgresql/errors_nl.properties,v
> retrieving revision 1.1
> diff -c -r1.1 errors_nl.properties
> *** src/interfaces/jdbc/org/postgresql/errors_nl.properties 2000/04/26 05:39:32 1.1
> --- src/interfaces/jdbc/org/postgresql/errors_nl.properties 2001/08/26 18:20:15
> ***************
> *** 54,60 ****
> postgresql.serial.noclass:Geen class gevonden voor {0}.
>
> postgresql.serial.table:De tabel voor {0} is niet in de database. Neem contact op met de DBA, omdat de database in een inconsistente staat verkeert.
>
> postgresql.serial.underscore:Class namen mogen geen _ in zich hebben. Jij voerde {0} in.
>
> - postgresql.stat.batch.empty:De batch is leeg. Er is niets om uit te voeren.
>
> postgresql.stat.batch.error:Batch invoer {0} {1} werd afgebroken.
>
> postgresql.stat.maxfieldsize:Een poging om setMaxFieldSize() faalde - compiletime standaardwaarde van kracht.
>
> postgresql.stat.noresult:Geen resultaten werden teruggegeven door de query.
>
> --- 54,59 ----
> Index: src/interfaces/jdbc/org/postgresql/jdbc2/DatabaseMetaData.java
> ===================================================================
> RCS file: /home/projects/pgsql/cvsroot/pgsql/src/interfaces/jdbc/org/postgresql/jdbc2/DatabaseMetaData.java,v
> retrieving revision 1.31
> diff -c -r1.31 DatabaseMetaData.java
> *** src/interfaces/jdbc/org/postgresql/jdbc2/DatabaseMetaData.java 2001/08/24 16:50:17 1.31
> --- src/interfaces/jdbc/org/postgresql/jdbc2/DatabaseMetaData.java 2001/08/26 18:20:15
> ***************
> *** 2836,2842 ****
> }
>
> /**
> ! * New in 7.1 - If this is for PreparedStatement yes, ResultSet no
> */
> public boolean supportsBatchUpdates() throws SQLException
> {
> --- 2836,2842 ----
> }
>
> /**
> ! * Indicates whether the driver supports batch updates.
> */
> public boolean supportsBatchUpdates() throws SQLException
> {
> Index: src/interfaces/jdbc/org/postgresql/jdbc2/Statement.java
> ===================================================================
> RCS file: /home/projects/pgsql/cvsroot/pgsql/src/interfaces/jdbc/org/postgresql/jdbc2/Statement.java,v
> retrieving revision 1.11
> diff -c -r1.11 Statement.java
> *** src/interfaces/jdbc/org/postgresql/jdbc2/Statement.java 2001/08/10 14:42:07 1.11
> --- src/interfaces/jdbc/org/postgresql/jdbc2/Statement.java 2001/08/26 18:20:17
> ***************
> *** 179,198 ****
>
> public int[] executeBatch() throws SQLException
> {
> ! if(batch==null || batch.isEmpty())
> ! throw new PSQLException("postgresql.stat.batch.empty");
> !
> 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));
> }
> return result;
> }
> --- 179,204 ----
>
> public int[] executeBatch() throws SQLException
> {
> ! if(batch==null)
> ! batch=new Vector();
> int size=batch.size();
> int[] result=new int[size];
> int i=0;
> try {
> for(i=0;i<size;i++)
> result[i]=this.executeUpdate((String)batch.elementAt(i));
> } catch(SQLException e) {
> ! 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;
> }
> Index: src/interfaces/jdbc/org/postgresql/test/JDBC2Tests.java
> ===================================================================
> RCS file: /home/projects/pgsql/cvsroot/pgsql/src/interfaces/jdbc/org/postgresql/test/JDBC2Tests.java,v
> retrieving revision 1.4
> diff -c -r1.4 JDBC2Tests.java
> *** src/interfaces/jdbc/org/postgresql/test/JDBC2Tests.java 2001/07/21 18:52:11 1.4
> --- src/interfaces/jdbc/org/postgresql/test/JDBC2Tests.java 2001/08/26 18:20:17
> ***************
> *** 205,210 ****
> --- 205,214 ----
> suite.addTestSuite(TimestampTest.class);
>
> // PreparedStatement
> + suite.addTestSuite(BatchExecuteTest.class);
> +
> + // BatchExecute
> +
>
> // MetaData
>
> Index: src/interfaces/jdbc/org/postgresql/util/PSQLException.java
> ===================================================================
> RCS file: /home/projects/pgsql/cvsroot/pgsql/src/interfaces/jdbc/org/postgresql/util/PSQLException.java,v
> retrieving revision 1.4
> diff -c -r1.4 PSQLException.java
> *** src/interfaces/jdbc/org/postgresql/util/PSQLException.java 2001/01/25 09:16:36 1.4
> --- src/interfaces/jdbc/org/postgresql/util/PSQLException.java 2001/08/26 18:20:17
> ***************
> *** 2,9 ****
>
> import java.io.*;
> import java.sql.*;
> - import java.text.*;
> - import java.util.*;
>
> /**
> * This class extends SQLException, and provides our internationalisation handling
> --- 2,7 ----
> ***************
> *** 12,20 ****
> {
> private String message;
>
> - // Cache for future errors
> - static ResourceBundle bundle;
> -
> /**
> * This provides the same functionality to SQLException
> * @param error Error string
> --- 10,15 ----
> ***************
> *** 86,122 ****
> 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;
> ! }
> }
>
> - 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
> */
> --- 81,90 ----
> translate(error,argv);
> }
>
> ! private void translate(String error, Object[] args) {
> ! message = MessageTranslator.translate(error,args);
> }
>
> /**
> * Overides Throwable
> */
> ***************
> *** 140,144 ****
> {
> return message;
> }
> -
> }
> --- 108,111 ----
>
>
> ------------------------------------------------------------------------
>
> 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
> */
>
>
> ------------------------------------------------------------------------
>
> 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;
> }
> }
>
>
> ------------------------------------------------------------------------
>
> package org.postgresql.jdbc2;
>
> import org.postgresql.util.*;
> import java.sql.*;
>
> /**
> * This class extends java.sql.BatchUpdateException, and provides our
> * internationalisation handling.
> */
> class PBatchUpdateException extends java.sql.BatchUpdateException {
>
> private String message;
>
> public PBatchUpdateException(
> String error, Object arg1, Object arg2, int[] updateCounts ) {
>
> super(updateCounts);
>
> Object[] argv = new Object[2];
> argv[0] = arg1;
> argv[1] = arg2;
> translate(error,argv);
> }
>
> private void translate(String error, Object[] args) {
> message = MessageTranslator.translate(error,args);
> }
>
> // Overides Throwable
> public String getLocalizedMessage()
> {
> return message;
> }
>
> // Overides Throwable
> public String getMessage()
> {
> return message;
> }
>
> // Overides Object
> public String toString()
> {
> return message;
> }
> }
>
>
> ------------------------------------------------------------------------
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo(at)postgresql(dot)org so that your
> message can get through to the mailing list cleanly
>
> patch.diff
>
> Content-Type:
>
> text/plain
> Content-Encoding:
>
> quoted-printable
>
>
> ------------------------------------------------------------------------
> BatchExecuteTest.java
>
> Content-Type:
>
> text/plain
> Content-Encoding:
>
> quoted-printable
>
>
> ------------------------------------------------------------------------
> MessageTranslator.java
>
> Content-Type:
>
> text/plain
> Content-Encoding:
>
> quoted-printable
>
>
> ------------------------------------------------------------------------
> PBatchUpdateException.java
>
> Content-Type:
>
> text/plain
> Content-Encoding:
>
> quoted-printable
>
>
> ------------------------------------------------------------------------
> Part 1.6
>
> Content-Type:
>
> text/plain
> Content-Encoding:
>
> binary
>
>