Changeset 5881

Timestamp:

Nov 21, 2011, 11:54:26 AM (11 years ago)

Author:

Nicklas Nordborg

Message:

Fixes #1651: Error handling problems when using PostgreSQL

Adds the possibitlity to use "failSafe" query execution. This can only be enabled when we do our own SQL execution and not via Hibernate. The "failSafe" option should be enabled when the gui must be able to continue in order to render a useful html page. I have enabled this option on the spot data list page, but there may be other places that need it as well (eg. experiment explorer).

Location:

trunk

Files:

• src/clients/web/net/sf/basedb/clients/web/Base.java (modified) (1 diff)
• src/core/net/sf/basedb/core/AbstractQuery.java (modified) (2 diffs)
• src/core/net/sf/basedb/core/AbstractSqlQuery.java (modified) (7 diffs)
• src/core/net/sf/basedb/core/ReporterPropertyExpression.java (modified) (1 diff)
• src/core/net/sf/basedb/core/dbengine/AbstractDbEngine.java (modified) (1 diff)
• src/core/net/sf/basedb/core/dbengine/DbEngine.java (modified) (1 diff)
• src/core/net/sf/basedb/core/dbengine/PostgresDbEngine.java (modified) (1 diff)
• src/core/net/sf/basedb/core/hibernate/ResultSetWork.java (modified) (2 diffs)
• src/core/net/sf/basedb/core/hibernate/SavePointWork.java (added)
• src/core/net/sf/basedb/core/query/Query.java (modified) (1 diff)
• www/views/experiments/spotdata/list_spotdata.jsp (modified) (1 diff)

trunk/src/clients/web/net/sf/basedb/clients/web/Base.java

@@ -969 +969 @@
       query.setItemPermission(mode.forceItemPermission());
     }
+    query.setFailSafe(true);
     return query;
   }

trunk/src/core/net/sf/basedb/core/AbstractQuery.java

@@ -194 +194 @@
   
   /**
+    If transactions must be able to continue after an SQL failure or not.
+  */
+  private boolean failSafe;
+  
+  /**
     Create a new query.
     @param rootEntity The root entity of the query
@@ -460 +465 @@
     return getMainQuery(dc, true);
   }
+  
+  @Override
+  public void setFailSafe(boolean failSafe)
+  {
+    this.failSafe = failSafe;
+  }
+
+  @Override
+  public boolean isFailSafe()
+  {
+    return failSafe;
+  }
+
   // -------------------------------------------
 

trunk/src/core/net/sf/basedb/core/AbstractSqlQuery.java

@@ -25 +25 @@
 
 import net.sf.basedb.core.hibernate.ResultSetWork;
+import net.sf.basedb.core.hibernate.SavePointWork;
 import net.sf.basedb.core.query.QueryParameter;
 import net.sf.basedb.core.query.SqlQuery;
@@ -41 +42 @@
 import java.sql.ResultSet;
 import java.sql.SQLException;
+import java.sql.Savepoint;
+import java.sql.Statement;
 
 /**
@@ -103 +106 @@
   {
     long totalCount = -1;
-    // Get query string
+    CountWork work = createCountWork(dc);
+    try
+    {
+      if (debugSqlEnabled) logSql.debug("Executing count query: " + work.getSql());
+      boolean useSavePoint = isFailSafe() && 
+          HibernateUtil.getDbEngine().useSavePointToContinueTransactionFromSqlFailure();
+      totalCount = HibernateUtil.doJdbcWork(dc.getHibernateSession(), 
+        useSavePoint ? new SavePointWork<Long>(work) : work);
+    }
+    catch (SQLException ex)
+    {
+      logSql.error(work.getSql(), ex);
+      throw new BaseException(ex);
+    }
+    return totalCount;
+  }
+  // -------------------------------------------
+  
+  /**
+    Creates a work item for counting the number of total hits by the query.
+  */
+  private CountWork createCountWork(DbControl dc)
+  {
     String countSql = getCountQuery(dc, true);
-    // Parse SQL and replace named parameters with ?
-    final List<String> countParameterOrder = new LinkedList<String>();
-    final AbstractSqlQuery query = this;
+    List<String> countParameterOrder = new LinkedList<String>();
     countSql = parseParameters(countSql, countParameterOrder);
-    try
-    {
-      if (debugSqlEnabled) logSql.debug("Executing count query: " + countSql);
-      
-      totalCount = HibernateUtil.doJdbcWork(dc.getHibernateSession(), 
-        new ResultSetWork<Long>(dc.getSessionControl(), countSql)
-        {
-          @Override
-          protected void setParameters(PreparedStatement ps)
-            throws SQLException
-          {
-            query.setParameters(ps, countParameterOrder);
-          }
-          @Override
-          protected Long getResult(ResultSet rs)
-            throws SQLException
-          {
-            return rs.next() ? rs.getLong(1) : 0;
-          }
-        }
-      );
-    }
-    catch (SQLException ex)
-    {
-      logSql.error(countSql, ex);
-      throw new BaseException(ex);
-    }
-    return totalCount;
-  }
-  // -------------------------------------------
+    return new CountWork(dc.getSessionControl(), countSql, countParameterOrder);
+  }
   
   abstract Select[] getDefaultSelects();
@@ -171 +166 @@
     
     // Get the main query
-    String sql = getMainQuery(dc, true);
+    String mainSql = getMainQuery(dc, true);
     
     // Holds the names of the parameters in the query in the order they appear
@@ -177 +172 @@
     
     // Parse out named parameters
-    sql = parseParameters(sql, parameterOrder);
+    mainSql = parseParameters(mainSql, parameterOrder);
     
     // Add limit parameters
-    sql = bindLimits(sql, parameterOrder);
+    mainSql = bindLimits(mainSql, parameterOrder);
     // Main query is now built
     
-    // Load the total count if it is requested
-    if (isReturningTotalCount())
-    {
-      totalCount = count(dc);
-    }
-
     ResultSet result = null;
+    Connection conn = null;
+    Savepoint savepoint = null;
+    PreparedStatement ps = null;
+    RuntimeException exception = null;
+    String currentSql = null;
     try
     {
-      Connection c = HibernateUtil.getConnection(dc.getHibernateSession());
-      if (debugSqlEnabled) logSql.debug("Executing main query: " + sql);
-      PreparedStatement ps = c.prepareStatement(sql);
+      conn = HibernateUtil.getConnection(dc.getHibernateSession());   
+      if (isFailSafe() && 
+        HibernateUtil.getDbEngine().useSavePointToContinueTransactionFromSqlFailure())
+      {
+        savepoint = conn.setSavepoint();
+      }
+
+      // Load the total count if it is requested
+      if (isReturningTotalCount())
+      {
+        CountWork countWork = createCountWork(dc);
+        currentSql = countWork.getSql();
+        if (debugSqlEnabled) logSql.debug("Executing count query: " + currentSql);
+        countWork.execute(conn);
+        totalCount = countWork.getResult();
+      }
+      
+      if (debugSqlEnabled) logSql.debug("Executing main query: " + mainSql);
+      currentSql = mainSql;
+      ps = conn.prepareStatement(mainSql);
       setParameters(ps, parameterOrder);
       result = QueryExecutor.executeQuery(ps, dc.getSessionControl());
@@ -204 +215 @@
     catch (InterruptedException ex)
     {
-      throw new SignalException("Aborted by user.", ex);
+      exception = new SignalException("Aborted by user.", ex);
     }
     catch (SQLException ex)
     {
-      logSql.error(sql, ex);
-      throw new BaseException(ex);
+      logSql.error(currentSql, ex);
+      exception = new BaseException(ex);
+    }
+    finally
+    {
+      if (exception != null)
+      {
+        if (result != null) closeResultSet(result, currentSql);
+        if (ps != null) closeStatement(ps, currentSql);
+        if (savepoint != null) rollbackToSavepoint(conn, savepoint, currentSql);
+        throw exception;
+      }
+      else
+      {
+        releaseSavepoint(conn, savepoint, currentSql);
+      }
     }
     return new DynamicResultIterator(result, totalCount, getMaxResults());
@@ -367 +392 @@
   }
   
+  /**
+    Close the result set without throwing an exception. If
+    the {@link ResultSet#close()} method throws an exception
+    it is logged together with the message.
+    @param msg A message to write to the log if the close fails
+    @param result The result set to close
+    @since 3.1
+  */
+  protected void closeResultSet(ResultSet result, String msg)
+  {
+    try
+    {
+      result.close();
+    }
+    catch (Throwable t)
+    {
+      logSql.warn(msg, t);
+    }
+  }
+  
+  /**
+    Close the statement without throwing an exception. If
+    the {@link Statement#close()} method throws an exception
+    it is logged together with the message.
+    @param msg A message to write to the log if the close fails
+    @param stmt The statement to close
+    @since 3.1
+  */
+  protected void closeStatement(Statement stmt, String msg)
+  {
+    try
+    {
+      stmt.close();
+    }
+    catch (Throwable t)
+    {
+      logSql.warn(msg, t);
+    }
+  }
+  
+  /**
+    Rollback the current transaction to the given savepoint.
+    @param conn The connection to rollback
+    @param savepoint The savepoint to rollback to
+    @param msg A message to write to the log if the rollback fails
+    @since 3.1
+  */
+  protected void rollbackToSavepoint(Connection conn, Savepoint savepoint, String msg)
+  {
+    try
+    {
+      conn.rollback(savepoint);
+    }
+    catch (Throwable t)
+    {
+      logSql.warn(msg, t);
+    }
+  }
+
+  /**
+    Release the given savepoint.
+    @param conn The connection that holds the savepoint
+    @param savepoint The savepoint to release
+    @param msg A message to write to the log if the release fails
+    @since 3.1
+  */
+  protected void releaseSavepoint(Connection conn, Savepoint savepoint, String msg)
+  {
+    try
+    {
+      conn.releaseSavepoint(savepoint);
+    }
+    catch (Throwable t)
+    {
+      logSql.warn(msg, t);
+    }
+  }
+  
+  
+  class CountWork
+    extends ResultSetWork<Long>
+  {
+
+    
+    private List<String> parameterOrder;
+    
+    CountWork(SessionControl sc, String sql, List<String> parameterOrder)
+    {
+      super(sc, sql);
+      this.parameterOrder = parameterOrder;
+    }
+
+
+    @Override
+    protected void setParameters(PreparedStatement ps)
+      throws SQLException
+    {
+      AbstractSqlQuery.this.setParameters(ps, parameterOrder);
+    }
+
+    @Override
+    protected Long getResult(ResultSet rs)
+      throws SQLException
+    {
+      return rs.next() ? rs.getLong(1) : 0;
+    }
+
+    
+    
+  }
 }

trunk/src/core/net/sf/basedb/core/ReporterPropertyExpression.java

@@ -101 +101 @@
         if (useCloned(dynamicQuery)) 
         {
-          if (!dynamicQuery.getVirtualDb().hasClonedReporterProperty(property))
+          if (!dc.loadItem(dynamicQuery.getVirtualDb()).hasClonedReporterProperty(property))
           {
             throw new InvalidDataException("Reporter property is not cloned: " + property);

trunk/src/core/net/sf/basedb/core/dbengine/AbstractDbEngine.java

@@ -218 +218 @@
     return "\"" + name + "\"";
   }
+  /**
+    @return Always FALSE, but may be overridden by subclasses
+   */
+  @Override
+  public boolean useSavePointToContinueTransactionFromSqlFailure() 
+  {
+    return false;
+  }
   // -------------------------------------------
 

trunk/src/core/net/sf/basedb/core/dbengine/DbEngine.java

@@ -370 +370 @@
   public String inspectSchemaGenerationSQL(String sql, Dialect dialect, SchemaGenerator.Mode mode);
   
+  /**
+    If the underlying database need to create a savepoint before executing
+    an SQL statement that results in an error in order to be able to
+    continue using the same transaction for other SQL queries. This is,
+    for example, needed by PostgreSQL which will otherwise ignore all
+    SQL statements executed after the one that caused an error.
+    @return TRUE if a SAVEPOINT is needed, FALSE if not
+  */
+  public boolean useSavePointToContinueTransactionFromSqlFailure();
+  
 }

trunk/src/core/net/sf/basedb/core/dbengine/PostgresDbEngine.java

@@ -277 +277 @@
     return sql;
   }
+  /**
+    @return Always TRUE
+   */
+  @Override
+  public boolean useSavePointToContinueTransactionFromSqlFailure() 
+  {
+    return true;
+  }
   // -------------------------------------------
+
   
 

trunk/src/core/net/sf/basedb/core/hibernate/ResultSetWork.java

@@ -50 +50 @@
 {
 
-  private SessionControl sc;
-  private String sql;
+  private final SessionControl sc;
+  private final String sql;
   private R result;
   
@@ -115 +115 @@
 
   /**
+    The that is executed by this work.
+    @since 3.1
+  */
+  public String getSql()
+  {
+    return sql;
+  }
+  
+  /**
     This method should be overridden by subclasses that needs to
     set parameters on the statement that is going to be executed.

trunk/src/core/net/sf/basedb/core/query/Query.java

@@ -374 +374 @@
   */
   public String toQl(DbControl dc);
+  
+  /**
+    Set this flag to ensure that the transaction used to execute the query 
+    is allowed to continue even after a failure at the SQL level of this query.
+    Setting this flag to TRUE should allow the transaction to continue, setting 
+    it to FALSE may or may not allow it to continue. The default setting is FALSE.
+    
+    @param failSafe TRUE to ensure that the transaction is allowed to continue,
+      FALSE if it doesn't matter
+    @since 3.1
+  */
+  public void setFailSafe(boolean failSafe);
+
+  /**
+    Check if a failure of this query should still allow the current transaction
+    to continue.
+    @return TRUE if the transaction should be allowed to continue,
+      FALSE if it doesn't  matter
+    @since 3.1
+  */
+  public boolean isFailSafe();
+  
 }

trunk/www/views/experiments/spotdata/list_spotdata.jsp

@@ -119 +119 @@
     final DynamicSpotQuery query = bioAssay != null ? bioAssay.getSpotData() : bioAssaySet.getSpotData();
     cc.configureQuery(dc, query, selected.selectedProperties);
+    query.setFailSafe(true);
     spotData = query.iterate(dc);
   }