Return-Path: Delivered-To: apmail-cocoon-dev-archive@www.apache.org Received: (qmail 19629 invoked from network); 1 Mar 2006 06:26:30 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (209.237.227.199) by minotaur.apache.org with SMTP; 1 Mar 2006 06:26:30 -0000 Received: (qmail 76560 invoked by uid 500); 1 Mar 2006 06:27:11 -0000 Delivered-To: apmail-cocoon-dev-archive@cocoon.apache.org Received: (qmail 76285 invoked by uid 500); 1 Mar 2006 06:27:09 -0000 Mailing-List: contact dev-help@cocoon.apache.org; run by ezmlm Precedence: bulk list-help: list-unsubscribe: List-Post: Reply-To: dev@cocoon.apache.org List-Id: Delivered-To: mailing list dev@cocoon.apache.org Received: (qmail 75989 invoked by uid 99); 1 Mar 2006 06:27:08 -0000 Received: from [192.87.106.226] (HELO ajax.apache.org) (192.87.106.226) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 28 Feb 2006 22:27:08 -0800 Received: from ajax.apache.org (ajax.apache.org [127.0.0.1]) by ajax.apache.org (Postfix) with ESMTP id F2942E1 for ; Wed, 1 Mar 2006 07:26:46 +0100 (CET) Message-ID: <1827885637.1141194406991.JavaMail.jira@ajax.apache.org> Date: Wed, 1 Mar 2006 07:26:46 +0100 (CET) From: "David Crossley (JIRA)" To: dev@cocoon.apache.org Subject: [jira] Updated: (COCOON-719) [PATCH] Support for transactions in SQLTransformer MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Virus-Checked: Checked by ClamAV on apache.org X-Spam-Rating: minotaur.apache.org 1.6.2 0/1000/N [ http://issues.apache.org/jira/browse/COCOON-719?page=all ] David Crossley updated COCOON-719: ---------------------------------- Bugzilla Id: (was: 20631) Other Info: [Patch available] Description: I asked about transaction support in the SQLTransformer and got some good suggestions about using some DB specific SQL. I might have gone that way (we use Oracle 9i), but I wanted something more portable. Daniel Fagerstrom posted about a patch he wrote that allowed the SQLTransformer to share a connection among top level queries. Great, half way there! (Here's the link for that patch... http://issues.apache.org/bugzilla/show_bug.cgi?id=16718). I added in code to support setting a parameter like this; I put the code in the SQLTransformer to turn off the auto-commit flag and to issue the appropriate commit/rollback calls (on error). It works for a couple inserts I threw together. If I put an error in the second insert, the first is rolled back. If both are good, they both are commited. :-) I'm attaching the revised SQLTransformer. I know you guys like to see patches, so I attached that also. For now, this code has Daniel's patch applied (onto the 2.0.4 version) and my additions are bounded by comments (i.e. // DAK: Transaction and // DAK) Is this something that can be put into the scratchpad? We could give it a different package so people could just choose it in the sitemap.xmap Here is the context diff with the 2.0.4 SQLTransformer (I added comments surrounding my new code to make it easy to spot .... *** SQLTransformer.java Sun May 18 17:48:50 2003 --- SQLTransformer.java.orig Tue May 13 22:20:36 2003 *************** *** 102,110 **** public static final String MAGIC_PASSWORD = "password"; public static final String MAGIC_NR_OF_ROWS = "show-nr-of-rows"; public static final String MAGIC_QUERY = "query"; - // DAK: Transaction - public static final String MAGIC_TRANSACTION = "enable-transaction"; - // DAK public static final String MAGIC_VALUE = "value"; public static final String MAGIC_DOC_ELEMENT = "doc-element"; public static final String MAGIC_ROW_ELEMENT = "row-element"; --- 102,107 ---- *************** *** 186,194 **** protected XMLDeserializer interpreter; protected Parser parser; - /** The connection used by all top level queries */ - protected Connection conn; - /** * Constructor */ --- 183,188 ---- *************** *** 220,237 **** */ public void recycle() { super.recycle(); - try { - // Close the connection used by all top level queries - if (this.conn != null) { - // DAK: Transaction - this.conn.commit(); - // DAK - this.conn.close(); - this.conn = null; - } - } catch ( SQLException e ) { - getLogger().warn( "Could not close the connection", e ); - } this.queries.clear(); this.outUri = null; this.outPrefix = null; --- 214,219 ---- *************** *** 292,300 **** getLogger().debug( "ROW-ELEMENT: " + parameters.getParameter( SQLTransformer.MAGIC_ROW_ELEMENT, "row" ) ); getLogger().debug( "NS-URI: " + parameters.getParameter( SQLTransformer.MAGIC_NS_URI_ELEMENT, NAMESPACE ) ); getLogger().debug( "NS-PREFIX: " + parameters.getParameter( SQLTransformer.MAGIC_NS_PREFIX_ELEMENT, "" ) ); - // DAK: Transaction - getLogger().debug( "TRANSACTION: " + parameters.getParameter( SQLTransformer.MAGIC_TRANSACTION, "" ) ); - // DAK } } --- 274,279 ---- *************** *** 317,335 **** AttributesImpl attr = new AttributesImpl(); Query query = (Query) queries.elementAt( index ); boolean query_failure = false; - Connection conn = null; try { try { - if (index == 0) { - if (this.conn == null) // The first top level execute-query - this.conn = query.getConnection(); - // reuse the global connection for all top level queries - conn = this.conn; - } - else // index > 0, sub queries are always executed in an own connection - conn = query.getConnection(); - - query.setConnection(conn); query.execute(); } catch ( SQLException e ) { if (getLogger().isDebugEnabled()) { --- 296,303 ---- *************** *** 372,403 **** this.end( query.rowset_name ); } - // DAK: Transaction - else { - if (conn.getAutoCommit() == false) - conn.rollback(); - } - // DAK } catch ( SQLException e ) { if (getLogger().isDebugEnabled()) { getLogger().debug( "SQLTransformer.executeQuery()", e ); } - // DAK: Transaction - try { - if (conn.getAutoCommit() == false) - conn.rollback(); - } catch (SQLException ex) { - if (getLogger().isDebugEnabled()) { - getLogger().debug( "SQLTransformer.executeQuery()", e ); - } - } - // DAK throw new SAXException( e ); } finally { try { query.close(); - if (index > 0) // close the connection used by a sub query - conn.close(); } catch ( SQLException e ) { getLogger().warn( "SQLTransformer: Could not close JDBC connection", e ); } --- 340,353 ---- *************** *** 1018,1027 **** } } - protected void setConnection(Connection conn) { - this.conn = conn; - } - /** Get a Connection. Made this a separate method to separate the logic from the actual execution. */ protected Connection getConnection() throws SQLException { Connection result = null; --- 968,973 ---- *************** *** 1074,1088 **** password ); } } - // DAK: Transaction - String transaction = properties.getParameter( SQLTransformer.MAGIC_TRANSACTION, null ); - if (transaction != null || transaction.trim().toLowerCase().equals("true")) { - result.setAutoCommit(false); - if (getLogger().isDebugEnabled()) { - getLogger().debug("So, someone fetched a connection and transactions are enabled..."); - } - } - // DAK } catch ( SQLException e ) { transformer.getTheLogger().error( "Caught a SQLException", e ); throw e; --- 1020,1025 ---- *************** *** 1092,1101 **** } protected void execute() throws SQLException { - if (this.conn == null) { - throw new SQLException("A connection must be set before executing a query"); - } - this.rowset_name = properties.getParameter( SQLTransformer.MAGIC_DOC_ELEMENT, "rowset" ); this.row_name = properties.getParameter( SQLTransformer.MAGIC_ROW_ELEMENT, "row" ); --- 1029,1034 ---- *************** *** 1123,1128 **** --- 1056,1063 ---- transformer.getTheLogger().debug( "EXECUTING " + query ); } + conn = getConnection(); + try { if ( !isstoredprocedure ) { if ( oldDriver ) { *************** *** 1155,1160 **** --- 1090,1098 ---- } catch ( SQLException e ) { transformer.getTheLogger().error( "Caught a SQLException", e ); throw e; + } finally { + conn.close(); + conn = null; // To make sure we don't use this connection again. } } *************** *** 1233,1238 **** --- 1171,1178 ---- cst.close(); cst = null; // Prevent using cst again. } finally { + if ( conn != null ) + conn.close(); conn = null; } } was: I asked about transaction support in the SQLTransformer and got some good suggestions about using some DB specific SQL. I might have gone that way (we use Oracle 9i), but I wanted something more portable. Daniel Fagerstrom posted about a patch he wrote that allowed the SQLTransformer to share a connection among top level queries. Great, half way there! (Here's the link for that patch... http://issues.apache.org/bugzilla/show_bug.cgi?id=16718). I added in code to support setting a parameter like this; I put the code in the SQLTransformer to turn off the auto-commit flag and to issue the appropriate commit/rollback calls (on error). It works for a couple inserts I threw together. If I put an error in the second insert, the first is rolled back. If both are good, they both are commited. :-) I'm attaching the revised SQLTransformer. I know you guys like to see patches, so I attached that also. For now, this code has Daniel's patch applied (onto the 2.0.4 version) and my additions are bounded by comments (i.e. // DAK: Transaction and // DAK) Is this something that can be put into the scratchpad? We could give it a different package so people could just choose it in the sitemap.xmap Here is the context diff with the 2.0.4 SQLTransformer (I added comments surrounding my new code to make it easy to spot .... *** SQLTransformer.java Sun May 18 17:48:50 2003 --- SQLTransformer.java.orig Tue May 13 22:20:36 2003 *************** *** 102,110 **** public static final String MAGIC_PASSWORD = "password"; public static final String MAGIC_NR_OF_ROWS = "show-nr-of-rows"; public static final String MAGIC_QUERY = "query"; - // DAK: Transaction - public static final String MAGIC_TRANSACTION = "enable-transaction"; - // DAK public static final String MAGIC_VALUE = "value"; public static final String MAGIC_DOC_ELEMENT = "doc-element"; public static final String MAGIC_ROW_ELEMENT = "row-element"; --- 102,107 ---- *************** *** 186,194 **** protected XMLDeserializer interpreter; protected Parser parser; - /** The connection used by all top level queries */ - protected Connection conn; - /** * Constructor */ --- 183,188 ---- *************** *** 220,237 **** */ public void recycle() { super.recycle(); - try { - // Close the connection used by all top level queries - if (this.conn != null) { - // DAK: Transaction - this.conn.commit(); - // DAK - this.conn.close(); - this.conn = null; - } - } catch ( SQLException e ) { - getLogger().warn( "Could not close the connection", e ); - } this.queries.clear(); this.outUri = null; this.outPrefix = null; --- 214,219 ---- *************** *** 292,300 **** getLogger().debug( "ROW-ELEMENT: " + parameters.getParameter( SQLTransformer.MAGIC_ROW_ELEMENT, "row" ) ); getLogger().debug( "NS-URI: " + parameters.getParameter( SQLTransformer.MAGIC_NS_URI_ELEMENT, NAMESPACE ) ); getLogger().debug( "NS-PREFIX: " + parameters.getParameter( SQLTransformer.MAGIC_NS_PREFIX_ELEMENT, "" ) ); - // DAK: Transaction - getLogger().debug( "TRANSACTION: " + parameters.getParameter( SQLTransformer.MAGIC_TRANSACTION, "" ) ); - // DAK } } --- 274,279 ---- *************** *** 317,335 **** AttributesImpl attr = new AttributesImpl(); Query query = (Query) queries.elementAt( index ); boolean query_failure = false; - Connection conn = null; try { try { - if (index == 0) { - if (this.conn == null) // The first top level execute-query - this.conn = query.getConnection(); - // reuse the global connection for all top level queries - conn = this.conn; - } - else // index > 0, sub queries are always executed in an own connection - conn = query.getConnection(); - - query.setConnection(conn); query.execute(); } catch ( SQLException e ) { if (getLogger().isDebugEnabled()) { --- 296,303 ---- *************** *** 372,403 **** this.end( query.rowset_name ); } - // DAK: Transaction - else { - if (conn.getAutoCommit() == false) - conn.rollback(); - } - // DAK } catch ( SQLException e ) { if (getLogger().isDebugEnabled()) { getLogger().debug( "SQLTransformer.executeQuery()", e ); } - // DAK: Transaction - try { - if (conn.getAutoCommit() == false) - conn.rollback(); - } catch (SQLException ex) { - if (getLogger().isDebugEnabled()) { - getLogger().debug( "SQLTransformer.executeQuery()", e ); - } - } - // DAK throw new SAXException( e ); } finally { try { query.close(); - if (index > 0) // close the connection used by a sub query - conn.close(); } catch ( SQLException e ) { getLogger().warn( "SQLTransformer: Could not close JDBC connection", e ); } --- 340,353 ---- *************** *** 1018,1027 **** } } - protected void setConnection(Connection conn) { - this.conn = conn; - } - /** Get a Connection. Made this a separate method to separate the logic from the actual execution. */ protected Connection getConnection() throws SQLException { Connection result = null; --- 968,973 ---- *************** *** 1074,1088 **** password ); } } - // DAK: Transaction - String transaction = properties.getParameter( SQLTransformer.MAGIC_TRANSACTION, null ); - if (transaction != null || transaction.trim().toLowerCase().equals("true")) { - result.setAutoCommit(false); - if (getLogger().isDebugEnabled()) { - getLogger().debug("So, someone fetched a connection and transactions are enabled..."); - } - } - // DAK } catch ( SQLException e ) { transformer.getTheLogger().error( "Caught a SQLException", e ); throw e; --- 1020,1025 ---- *************** *** 1092,1101 **** } protected void execute() throws SQLException { - if (this.conn == null) { - throw new SQLException("A connection must be set before executing a query"); - } - this.rowset_name = properties.getParameter( SQLTransformer.MAGIC_DOC_ELEMENT, "rowset" ); this.row_name = properties.getParameter( SQLTransformer.MAGIC_ROW_ELEMENT, "row" ); --- 1029,1034 ---- *************** *** 1123,1128 **** --- 1056,1063 ---- transformer.getTheLogger().debug( "EXECUTING " + query ); } + conn = getConnection(); + try { if ( !isstoredprocedure ) { if ( oldDriver ) { *************** *** 1155,1160 **** --- 1090,1098 ---- } catch ( SQLException e ) { transformer.getTheLogger().error( "Caught a SQLException", e ); throw e; + } finally { + conn.close(); + conn = null; // To make sure we don't use this connection again. } } *************** *** 1233,1238 **** --- 1171,1178 ---- cst.close(); cst = null; // Prevent using cst again. } finally { + if ( conn != null ) + conn.close(); conn = null; } } > [PATCH] Support for transactions in SQLTransformer > -------------------------------------------------- > > Key: COCOON-719 > URL: http://issues.apache.org/jira/browse/COCOON-719 > Project: Cocoon > Type: Improvement > Components: - Components: Sitemap > Versions: 2.0.4 > Environment: Operating System: All > Platform: All > Reporter: David Kavanagh > Assignee: Cocoon Developers Team > Priority: Minor > Attachments: SQLTransformer.java, SQLTransformer.java > > I asked about transaction support in the SQLTransformer and got some good > suggestions about using some DB specific SQL. I might have gone that way (we use > Oracle 9i), but I wanted something more portable. > Daniel Fagerstrom posted about a patch he wrote that allowed the SQLTransformer > to share a connection among top level queries. Great, half way there! (Here's > the link for that patch... http://issues.apache.org/bugzilla/show_bug.cgi?id=16718). > I added in code to support setting a parameter like this; > > > > > I put the code in the SQLTransformer to turn off the auto-commit flag and to > issue the appropriate commit/rollback calls (on error). It works for a couple > inserts I threw together. If I put an error in the second insert, the first is > rolled back. If both are good, they both are commited. :-) > I'm attaching the revised SQLTransformer. I know you guys like to see patches, > so I attached that also. For now, this code has Daniel's patch applied (onto the > 2.0.4 version) and my additions are bounded by comments (i.e. // DAK: > Transaction and // DAK) > Is this something that can be put into the scratchpad? We could give it a > different package so people could just choose it in the sitemap.xmap > Here is the context diff with the 2.0.4 SQLTransformer (I added comments > surrounding my new code to make it easy to spot > .... > *** SQLTransformer.java Sun May 18 17:48:50 2003 > --- SQLTransformer.java.orig Tue May 13 22:20:36 2003 > *************** > *** 102,110 **** > public static final String MAGIC_PASSWORD = "password"; > public static final String MAGIC_NR_OF_ROWS = "show-nr-of-rows"; > public static final String MAGIC_QUERY = "query"; > - // DAK: Transaction > - public static final String MAGIC_TRANSACTION = "enable-transaction"; > - // DAK > public static final String MAGIC_VALUE = "value"; > public static final String MAGIC_DOC_ELEMENT = "doc-element"; > public static final String MAGIC_ROW_ELEMENT = "row-element"; > --- 102,107 ---- > *************** > *** 186,194 **** > protected XMLDeserializer interpreter; > protected Parser parser; > > - /** The connection used by all top level queries */ > - protected Connection conn; > - > /** > * Constructor > */ > --- 183,188 ---- > *************** > *** 220,237 **** > */ > public void recycle() { > super.recycle(); > - try { > - // Close the connection used by all top level queries > - if (this.conn != null) { > - // DAK: Transaction > - this.conn.commit(); > - // DAK > - this.conn.close(); > - this.conn = null; > - } > - } catch ( SQLException e ) { > - getLogger().warn( "Could not close the connection", e ); > - } > this.queries.clear(); > this.outUri = null; > this.outPrefix = null; > --- 214,219 ---- > *************** > *** 292,300 **** > getLogger().debug( "ROW-ELEMENT: " + parameters.getParameter( > SQLTransformer.MAGIC_ROW_ELEMENT, "row" ) ); > getLogger().debug( "NS-URI: " + parameters.getParameter( > SQLTransformer.MAGIC_NS_URI_ELEMENT, NAMESPACE ) ); > getLogger().debug( "NS-PREFIX: " + parameters.getParameter( > SQLTransformer.MAGIC_NS_PREFIX_ELEMENT, "" ) ); > - // DAK: Transaction > - getLogger().debug( "TRANSACTION: " + parameters.getParameter( > SQLTransformer.MAGIC_TRANSACTION, "" ) ); > - // DAK > } > } > > --- 274,279 ---- > *************** > *** 317,335 **** > AttributesImpl attr = new AttributesImpl(); > Query query = (Query) queries.elementAt( index ); > boolean query_failure = false; > - Connection conn = null; > try { > try { > - if (index == 0) { > - if (this.conn == null) // The first top level execute-query > - this.conn = query.getConnection(); > - // reuse the global connection for all top level queries > - conn = this.conn; > - } > - else // index > 0, sub queries are always executed in an own connection > - conn = query.getConnection(); > - > - query.setConnection(conn); > query.execute(); > } catch ( SQLException e ) { > if (getLogger().isDebugEnabled()) { > --- 296,303 ---- > *************** > *** 372,403 **** > > this.end( query.rowset_name ); > } > - // DAK: Transaction > - else { > - if (conn.getAutoCommit() == false) > - conn.rollback(); > - } > - // DAK > } catch ( SQLException e ) { > if (getLogger().isDebugEnabled()) { > getLogger().debug( "SQLTransformer.executeQuery()", e ); > } > - // DAK: Transaction > - try { > - if (conn.getAutoCommit() == false) > - conn.rollback(); > - } catch (SQLException ex) { > - if (getLogger().isDebugEnabled()) { > - getLogger().debug( "SQLTransformer.executeQuery()", e ); > - } > - } > - // DAK > throw new SAXException( e ); > } finally { > try { > query.close(); > - if (index > 0) // close the connection used by a sub query > - conn.close(); > } catch ( SQLException e ) { > getLogger().warn( "SQLTransformer: Could not close JDBC > connection", e ); > } > --- 340,353 ---- > *************** > *** 1018,1027 **** > } > } > > - protected void setConnection(Connection conn) { > - this.conn = conn; > - } > - > /** Get a Connection. Made this a separate method to separate the > logic from the actual execution. */ > protected Connection getConnection() throws SQLException { > Connection result = null; > --- 968,973 ---- > *************** > *** 1074,1088 **** > password ); > } > } > - // DAK: Transaction > - String transaction = properties.getParameter( > SQLTransformer.MAGIC_TRANSACTION, null ); > - if (transaction != null || transaction.trim().toLowerCase().equals("true")) { > - result.setAutoCommit(false); > - if (getLogger().isDebugEnabled()) { > - getLogger().debug("So, someone fetched a connection and transactions are > enabled..."); > - } > - } > - // DAK > } catch ( SQLException e ) { > transformer.getTheLogger().error( "Caught a SQLException", e ); > throw e; > --- 1020,1025 ---- > *************** > *** 1092,1101 **** > } > > protected void execute() throws SQLException { > - if (this.conn == null) { > - throw new SQLException("A connection must be set before executing a query"); > - } > - > this.rowset_name = properties.getParameter( > SQLTransformer.MAGIC_DOC_ELEMENT, "rowset" ); > this.row_name = properties.getParameter( > SQLTransformer.MAGIC_ROW_ELEMENT, "row" ); > > --- 1029,1034 ---- > *************** > *** 1123,1128 **** > --- 1056,1063 ---- > transformer.getTheLogger().debug( "EXECUTING " + query ); > } > > + conn = getConnection(); > + > try { > if ( !isstoredprocedure ) { > if ( oldDriver ) { > *************** > *** 1155,1160 **** > --- 1090,1098 ---- > } catch ( SQLException e ) { > transformer.getTheLogger().error( "Caught a SQLException", e ); > throw e; > + } finally { > + conn.close(); > + conn = null; // To make sure we don't use this > connection again. > } > } > > *************** > *** 1233,1238 **** > --- 1171,1178 ---- > cst.close(); > cst = null; // Prevent using cst again. > } finally { > + if ( conn != null ) > + conn.close(); > conn = null; > } > } -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://issues.apache.org/jira/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira