[364809] DBStore with Postgres fail to recover from crash

https://bugs.eclipse.org/bugs/show_bug.cgi?id=364809

4 files changed, 101 insertions, 62 deletions

diff --git a/plugins/org.eclipse.emf.cdo.server.db/src/org/eclipse/emf/cdo/server/internal/db/DBStore.java b/plugins/org.eclipse.emf.cdo.server.db/src/org/eclipse/emf/cdo/server/internal/db/DBStore.java
index 0484959a21..ccada761dc 100644
--- a/plugins/org.eclipse.emf.cdo.server.db/src/org/eclipse/emf/cdo/server/internal/db/DBStore.java
+++ b/plugins/org.eclipse.emf.cdo.server.db/src/org/eclipse/emf/cdo/server/internal/db/DBStore.java
@@ -98,10 +98,6 @@ public class DBStore extends Store implements IDBStore, CDOAllRevisionsProvider
 
   private static final String PROP_GRACEFULLY_SHUT_DOWN = "org.eclipse.emf.cdo.server.db.gracefullyShutDown"; //$NON-NLS-1$
 
-  private static final String TABLE_NOT_FOUND = "42S02";
-
-  private static final String COLUMN_NOT_FOUND = "42S22";
-
   private long creationTime;
 
   private boolean firstTime;
@@ -276,7 +272,7 @@ public class DBStore extends Store implements IDBStore, CDOAllRevisionsProvider
           throw new DBException(ex1);
         }
 
-        if (!COLUMN_NOT_FOUND.equalsIgnoreCase(ex.getSQLState()))
+        if (!dbAdapter.isColumnNotFoundException(ex))
         {
           throw new DBException(ex);
         }
@@ -822,7 +818,7 @@ public class DBStore extends Store implements IDBStore, CDOAllRevisionsProvider
     }
     catch (SQLException ex)
     {
-      if (TABLE_NOT_FOUND.equals(ex.getSQLState()))
+      if (dbAdapter.isTableNotFoundException(ex))
       {
         return FIRST_START;
       }
@@ -839,12 +835,10 @@ public class DBStore extends Store implements IDBStore, CDOAllRevisionsProvider
   protected void migrateSchema(int fromVersion) throws Exception
   {
     Connection connection = null;
-    Statement statement = null;
 
     try
     {
       connection = getConnection();
-      statement = connection.createStatement();
 
       for (int version = fromVersion; version < SCHEMA_VERSION; version++)
       {
@@ -852,7 +846,7 @@ public class DBStore extends Store implements IDBStore, CDOAllRevisionsProvider
         {
           int nextVersion = version + 1;
           OM.LOG.info("Migrating schema from version " + version + " to version " + nextVersion + "...");
-          SCHEMA_MIGRATORS[version].migrateSchema(this, statement);
+          SCHEMA_MIGRATORS[version].migrateSchema(this, connection);
         }
       }
 
@@ -860,7 +854,6 @@ public class DBStore extends Store implements IDBStore, CDOAllRevisionsProvider
     }
     finally
     {
-      DBUtil.close(statement);
       DBUtil.close(connection);
     }
   }
@@ -870,7 +863,7 @@ public class DBStore extends Store implements IDBStore, CDOAllRevisionsProvider
    */
   private static abstract class SchemaMigrator
   {
-    public abstract void migrateSchema(DBStore store, Statement statement) throws Exception;
+    public abstract void migrateSchema(DBStore store, Connection connection) throws Exception;
   }
 
   private static final SchemaMigrator NO_MIGRATION_NEEDED = null;
@@ -878,22 +871,33 @@ public class DBStore extends Store implements IDBStore, CDOAllRevisionsProvider
   private static final SchemaMigrator NON_AUDIT_MIGRATION = new SchemaMigrator()
   {
     @Override
-    public void migrateSchema(DBStore store, final Statement statement) throws Exception
+    public void migrateSchema(DBStore store, Connection connection) throws Exception
     {
       InternalRepository repository = store.getRepository();
       if (!repository.isSupportingAudits())
       {
-        store.visitAllTables(statement.getConnection(), new IDBStore.TableVisitor()
+        store.visitAllTables(connection, new IDBStore.TableVisitor()
         {
           public void visitTable(Connection connection, String name) throws SQLException
           {
-            String from = " FROM " + name + " WHERE " + CDODBSchema.ATTRIBUTES_VERSION + "<"
-                + CDOBranchVersion.FIRST_VERSION;
+            Statement statement = null;
 
-            statement.executeUpdate("DELETE FROM " + CDODBSchema.CDO_OBJECTS + " WHERE " + CDODBSchema.ATTRIBUTES_ID
-                + " IN (SELECT " + CDODBSchema.ATTRIBUTES_ID + from + ")");
+            try
+            {
+              statement = connection.createStatement();
+
+              String from = " FROM " + name + " WHERE " + CDODBSchema.ATTRIBUTES_VERSION + "<"
+                  + CDOBranchVersion.FIRST_VERSION;
 
-            statement.executeUpdate("DELETE" + from);
+              statement.executeUpdate("DELETE FROM " + CDODBSchema.CDO_OBJECTS + " WHERE " + CDODBSchema.ATTRIBUTES_ID
+                  + " IN (SELECT " + CDODBSchema.ATTRIBUTES_ID + from + ")");
+
+              statement.executeUpdate("DELETE" + from);
+            }
+            finally
+            {
+              DBUtil.close(statement);
+            }
           }
         });
       }
@@ -903,11 +907,22 @@ public class DBStore extends Store implements IDBStore, CDOAllRevisionsProvider
   private static final SchemaMigrator LOB_SIZE_MIGRATION = new SchemaMigrator()
   {
     @Override
-    public void migrateSchema(DBStore store, final Statement statement) throws Exception
+    public void migrateSchema(DBStore store, final Connection connection) throws Exception
     {
-      IDBAdapter dbAdapter = store.getDBAdapter();
-      String sql = dbAdapter.sqlRenameField(CDODBSchema.LOBS_SIZE, "size");
-      statement.execute(sql);
+      Statement statement = null;
+
+      try
+      {
+        statement = connection.createStatement();
+
+        IDBAdapter dbAdapter = store.getDBAdapter();
+        String sql = dbAdapter.sqlRenameField(CDODBSchema.LOBS_SIZE, "size");
+        statement.execute(sql);
+      }
+      finally
+      {
+        DBUtil.close(statement);
+      }
     }
   };
 
diff --git a/plugins/org.eclipse.net4j.db.postgresql/src/org/eclipse/net4j/db/postgresql/PostgreSQLAdapter.java b/plugins/org.eclipse.net4j.db.postgresql/src/org/eclipse/net4j/db/postgresql/PostgreSQLAdapter.java
index 3ff7c75a9e..43614e8225 100644
--- a/plugins/org.eclipse.net4j.db.postgresql/src/org/eclipse/net4j/db/postgresql/PostgreSQLAdapter.java
+++ b/plugins/org.eclipse.net4j.db.postgresql/src/org/eclipse/net4j/db/postgresql/PostgreSQLAdapter.java
@@ -13,14 +13,12 @@
  */
 package org.eclipse.net4j.db.postgresql;
 
-import org.eclipse.net4j.db.DBException;
 import org.eclipse.net4j.db.DBType;
 import org.eclipse.net4j.db.IDBAdapter;
 import org.eclipse.net4j.db.ddl.IDBField;
 import org.eclipse.net4j.db.ddl.IDBTable;
 import org.eclipse.net4j.db.internal.postgresql.bundle.OM;
 import org.eclipse.net4j.spi.db.DBAdapter;
-import org.eclipse.net4j.util.om.trace.ContextTracer;
 
 import org.postgresql.ds.PGSimpleDataSource;
 
@@ -42,7 +40,7 @@ public class PostgreSQLAdapter extends DBAdapter
 
   public static final String VERSION = "9.0"; //$NON-NLS-1$
 
-  private static final ContextTracer TRACER = new ContextTracer(OM.DEBUG_SQL, DBAdapter.class);
+  // private static final ContextTracer TRACER = new ContextTracer(OM.DEBUG_SQL, DBAdapter.class);
 
   private static final String[] RESERVED_WORDS = { "ALL", "ANALYSE", "ANALYZE", "AND", "ANY", "AS", "ASC", "ATOMIC", //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ //$NON-NLS-5$ //$NON-NLS-6$ //$NON-NLS-7$ //$NON-NLS-8$
       "AUTHORIZATION", "BETWEEN", "BIGINT", "BINARY", "BIT", "BOOLEAN", "BOTH", "C", "CASE", "CAST", "CHAR", //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$ //$NON-NLS-5$ //$NON-NLS-6$ //$NON-NLS-7$ //$NON-NLS-8$ //$NON-NLS-9$ //$NON-NLS-10$ //$NON-NLS-11$
@@ -117,57 +115,58 @@ public class PostgreSQLAdapter extends DBAdapter
     return RESERVED_WORDS;
   }
 
-  /*
-   * TODO Remove this method override after fixing Bug 282791 - [DB] Check for existing tables instead of relying on
-   * SQLExceptions PostgreSQL uses transaction on DDL operations. If an error occurs, the SQL Connection goes to an
-   * error state, and can only be cleared by rolling back. Therefore, savepoints for table creation were added
+  /**
+   * See <a href="http://www.postgresql.org/docs/9.0/static/errcodes-appendix.html">Appendix A. PostgreSQL Error Codes</a>.
    */
   @Override
-  public boolean createTable(IDBTable table, Statement statement) throws DBException
+  public boolean isDuplicateKeyException(SQLException ex)
   {
-    boolean created = true;
-    Savepoint savepoint = null;
+    // RESTRICT VIOLATION || UNIQUE VIOLATION
+    return super.isDuplicateKeyException(ex) || "23505".equals(ex.getSQLState());
+  }
 
-    try
-    {
-      savepoint = statement.getConnection().setSavepoint();
-    }
-    catch (SQLException ex)
-    {
-      OM.LOG.error(ex);
-    }
+  /**
+   * See <a href="http://www.postgresql.org/docs/9.0/static/errcodes-appendix.html">Appendix A. PostgreSQL Error Codes</a>.
+   */
+  @Override
+  public boolean isTableNotFoundException(SQLException ex)
+  {
+    // UNDEFINED TABLE
+    return "42P01".equals(ex.getSQLState());
+  }
+
+  /**
+   * See <a href="http://www.postgresql.org/docs/9.0/static/errcodes-appendix.html">Appendix A. PostgreSQL Error Codes</a>.
+   */
+  @Override
+  public boolean isColumnNotFoundException(SQLException ex)
+  {
+    // UNDEFINED COLUMN
+    return "42703".equals(ex.getSQLState());
+  }
+
+  @Override
+  protected void doCreateTable(IDBTable table, Statement statement) throws SQLException
+  {
+    Savepoint savepoint = statement.getConnection().setSavepoint();
 
     try
     {
-      doCreateTable(table, statement);
+      super.doCreateTable(table, statement);
     }
     catch (SQLException ex)
     {
-      created = false;
-      if (TRACER.isEnabled())
-      {
-        TRACER.trace("-- " + ex.getMessage() + ". Trying to rollback operation"); //$NON-NLS-1$
-      }
-
-      if (savepoint != null)
+      try
       {
-        try
-        {
-          statement.getConnection().rollback(savepoint);
-        }
-        catch (SQLException ex1)
-        {
-          OM.LOG.error(ex1);
-        }
+        statement.getConnection().rollback(savepoint);
       }
-      else
+      catch (SQLException ex1)
       {
-        OM.LOG.error("Could not rollback last operation. Savepoint was not created."); //$NON-NLS-1$
+        OM.LOG.error(ex1);
       }
-    }
 
-    validateTable(table, statement);
-    return created;
+      throw ex;
+    }
   }
 
   @Override
diff --git a/plugins/org.eclipse.net4j.db/src/org/eclipse/net4j/db/IDBAdapter.java b/plugins/org.eclipse.net4j.db/src/org/eclipse/net4j/db/IDBAdapter.java
index 5d73ba2e2a..3aaa5a6fcc 100644
--- a/plugins/org.eclipse.net4j.db/src/org/eclipse/net4j/db/IDBAdapter.java
+++ b/plugins/org.eclipse.net4j.db/src/org/eclipse/net4j/db/IDBAdapter.java
@@ -103,5 +103,15 @@ public interface IDBAdapter
   /**
    * @since 4.2
    */
+  public boolean isTableNotFoundException(SQLException ex);
+
+  /**
+   * @since 4.2
+   */
+  public boolean isColumnNotFoundException(SQLException ex);
+
+  /**
+   * @since 4.2
+   */
   public String sqlRenameField(IDBField field, String oldName);
 }
diff --git a/plugins/org.eclipse.net4j.db/src/org/eclipse/net4j/spi/db/DBAdapter.java b/plugins/org.eclipse.net4j.db/src/org/eclipse/net4j/spi/db/DBAdapter.java
index e1405f1bca..9892e0fbd6 100644
--- a/plugins/org.eclipse.net4j.db/src/org/eclipse/net4j/spi/db/DBAdapter.java
+++ b/plugins/org.eclipse.net4j.db/src/org/eclipse/net4j/spi/db/DBAdapter.java
@@ -557,13 +557,28 @@ public abstract class DBAdapter implements IDBAdapter
    */
   public boolean isDuplicateKeyException(SQLException ex)
   {
-    /* SQL code for duplicate keys is 23001 */
     return "23001".equals(ex.getSQLState());
   }
 
   /**
    * @since 4.2
    */
+  public boolean isTableNotFoundException(SQLException ex)
+  {
+    return "42S02".equals(ex.getSQLState());
+  }
+
+  /**
+   * @since 4.2
+   */
+  public boolean isColumnNotFoundException(SQLException ex)
+  {
+    return "42S22".equals(ex.getSQLState());
+  }
+
+  /**
+   * @since 4.2
+   */
   public String sqlRenameField(IDBField field, String oldName)
   {
     return "ALTER TABLE " + field.getTable() + " RENAME COLUMN " + oldName + " TO " + field;