You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cocoon.apache.org by "David Crossley (JIRA)" <ji...@apache.org> on 2006/03/01 07:26:46 UTC
[jira] Updated: (COCOON-719) [PATCH] Support for transactions in
SQLTransformer
[ 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;
<map:transform type="sql">
<map:parameter name="use-connection" value="mbrdb"/>
<map:parameter name="enable-transaction" value="true"/>
</map:transform>
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;
<map:transform type="sql">
<map:parameter name="use-connection" value="mbrdb"/>
<map:parameter name="enable-transaction" value="true"/>
</map:transform>
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;
> <map:transform type="sql">
> <map:parameter name="use-connection" value="mbrdb"/>
> <map:parameter name="enable-transaction" value="true"/>
> </map:transform>
> 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