Cleanup #1

h2/src/main/org/h2/command/Parser.java

@@ -198,7 +198,7 @@ public class Parser {
             return o1 == o2 ? 0 : compareTableFilters(o1, o2);
         }
     };
-    public static final String WITH_STATEMENT_SUPPORTS_LIMITED_STATEMENTS =
+    public static final String WITH_STATEMENT_SUPPORTS_LIMITED_SUB_STATEMENTS =
             "WITH statement supports only SELECT, CREATE TABLE, INSERT, UPDATE, MERGE or DELETE statements";
 
     private final Database database;
@@ -1453,7 +1453,7 @@ public class Parser {
                 }
             }
         }
-        // inherit alias for temporary views (usually CTE's) or explicity CTEs from table name
+        // inherit alias for CTE as views from table name
         if(table.isView() && table.isTableExpression() && alias==null){
             alias = table.getName();
         }
@@ -5170,9 +5170,6 @@ public class Parser {
             Query query = parseSelectUnion();
             query.setPrepareAlways(true);
             query.setNeverLazy(true);
-            if(isPersistent){
-                query.session = session.getDatabase().getSystemSession();
-            }
             p = query;
         }
         else if(readIf("INSERT")) {
@@ -5194,7 +5191,7 @@ public class Parser {
         else if(readIf("CREATE")) {
             if (!isToken("TABLE")){
                 throw DbException.get(ErrorCode.SYNTAX_ERROR_1,
-                        WITH_STATEMENT_SUPPORTS_LIMITED_STATEMENTS);
+                        WITH_STATEMENT_SUPPORTS_LIMITED_SUB_STATEMENTS);
 
             }
             p = parseCreate();
@@ -5202,7 +5199,7 @@ public class Parser {
         }
         else {
             throw DbException.get(ErrorCode.SYNTAX_ERROR_1,
-                    WITH_STATEMENT_SUPPORTS_LIMITED_STATEMENTS);
+                    WITH_STATEMENT_SUPPORTS_LIMITED_SUB_STATEMENTS);
         }
 
         // clean up temporary views starting with last to first (in case of
@@ -5213,9 +5210,7 @@ public class Parser {
         return p;
     }
 
-    //@SuppressWarnings("resource")// Eclipse thinks targetSession needs releasing
     private TableView parseSingleCommonTableExpression(boolean isPersistent) {
-        //Session targetSession =  isPersistent ? database.getSystemSession() : session;
         Session targetSession = session;
         String cteViewName = readIdentifierWithSchema();
         Schema schema = getSchema();
@@ -5224,16 +5219,13 @@ public class Parser {
         String[] cols = null;
         Database db = targetSession.getDatabase();
         
-        //System.out.println("systemSessionId="+database.getSystemSession().getId());
-        //System.out.println("sessionId="+session.getId());
-
         // column names are now optional - they can be inferred from the named
         // query, if not supplied by user
         if (readIf("(")) {
             cols = parseColumnList();
             for (String c : cols) {
                 // we don't really know the type of the column, so STRING will
-                // have to do
+                // have to do, UNKNOWN does not work here
                 columns.add(new Column(c, Value.STRING));
             }
         }
@@ -5257,7 +5249,6 @@ public class Parser {
                         cteViewName);
             }
             if(isPersistent){
-                //System.out.println("parseSingleCommonTableExpression removeSchemaObject "+oldViewFound.getName());
                 oldViewFound.lock(targetSession, true, true);
                 targetSession.getDatabase().removeSchemaObject(targetSession, oldViewFound);                
                 
@@ -5293,7 +5284,8 @@ public class Parser {
 
         TableView view = createCTEView(cteViewName,
                 querySQLOutput[0], columnTemplateList,
-                true/* allowRecursiveQueryDetection */, true/* add to session */, 
+                true/* allowRecursiveQueryDetection */, 
+                true/* add to session */, 
                 isPersistent, targetSession);
         
         return view;
@@ -5309,7 +5301,8 @@ public class Parser {
             }else{
                 targetSession.removeLocalTempTable(recursiveTable);
             }
-            // both removeSchemaObject and removeLocalTempTable hold meta locks
+            
+            // both removeSchemaObject and removeLocalTempTable hold meta locks - release them here
             targetSession.getDatabase().unlockMeta(targetSession);
         }
     }
@@ -5384,6 +5377,7 @@ public class Parser {
         Schema schema = getSchemaWithDefault();
         int id = db.allocateObjectId();
         Column[] columnTemplateArray = columnTemplateList.toArray(new Column[0]);
+        
         // No easy way to determine if this is a recursive query up front, so we just compile
         // it twice - once without the flag set, and if we didn't see a recursive term,
         // then we just compile it again.

h2/src/main/org/h2/command/ddl/CreateView.java

@@ -137,6 +137,10 @@ public class CreateView extends SchemaCommand {
         } else {
             db.updateMeta(session, view);
         }
+        
+        // TODO: if we added any table expressions that aren't used by this view, detect them
+        // and drop them - otherwise they will leak and never get cleaned up.
+        
         return 0;
     }
 

h2/src/main/org/h2/command/ddl/DropTable.java

@@ -100,7 +100,6 @@ public class DropTable extends SchemaCommand {
             Database db = session.getDatabase();
             db.lockMeta(session);
             db.removeSchemaObject(session, table);
-            //session.getDatabase().flushDeferredRemoveSchemaObject();
         }
         if (next != null) {
             next.executeDrop();

h2/src/main/org/h2/command/ddl/DropView.java

@@ -69,24 +69,21 @@ public class DropView extends SchemaCommand {
                 }
             }
             
-            TableView tableView = (TableView) view;
-            ArrayList<Table> copyOfDependencies = new ArrayList<Table>(tableView.getTables());
-            
              // TODO: Where is the ConstraintReferential.CASCADE style drop processing ? It's
             // supported from imported keys - but not for dependent db objects            
 
+            TableView tableView = (TableView) view;
+            ArrayList<Table> copyOfDependencies = new ArrayList<Table>(tableView.getTables());
+            
             view.lock(session, true, true);
             session.getDatabase().removeSchemaObject(session, view);
             session.getDatabase().unlockMeta(session);
-            //session.getDatabase().flushDeferredRemoveSchemaObject();
 
             // remove dependent table expressions
             for( Table childTable: copyOfDependencies){
                 if(TableType.VIEW == childTable.getTableType()){
                     TableView childTableView = (TableView) childTable;
-                    //System.out.println("considering dep "+childTableView.getName());
                     if(childTableView.isTableExpression() && childTableView.getName()!=null){
-                        //System.out.println("removing "+childTableView.getName());
                         session.getDatabase().removeSchemaObject(session, childTableView);
                     }
                 }

h2/src/main/org/h2/command/dml/Query.java

@@ -7,6 +7,7 @@ package org.h2.command.dml;
 
 import java.util.ArrayList;
 import java.util.HashSet;
+
 import org.h2.api.ErrorCode;
 import org.h2.command.Prepared;
 import org.h2.engine.Database;

h2/src/main/org/h2/command/dml/Select.java

@@ -1090,6 +1090,8 @@ public class Select extends Query {
             if (tableView!=null && tableView.isView() && tableView.isRecursive() && tableView.isTableExpression()) {
                 
                 if(tableView.isPersistent()){
+                    // skip the generation of plan SQL for this already recursive persistent ctes, since using a with 
+                    // statement will re-create the common table expression views.
                     continue;
                 }
                 else {

h2/src/main/org/h2/engine/Database.java

@@ -11,8 +11,6 @@ import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
-//import java.util.Iterator;
-//import java.util.Map.Entry;
 import java.util.Properties;
 import java.util.Set;
 import java.util.StringTokenizer;
@@ -204,7 +202,6 @@ public class Database implements DataHandler {
     private int queryStatisticsMaxEntries = Constants.QUERY_STATISTICS_MAX_ENTRIES;
     private QueryStatisticsData queryStatisticsData;
     private RowFactory rowFactory = RowFactory.DEFAULT;
-//    private ConcurrentHashMap<SchemaObject, Session> removeSchemaObjectQueue = new ConcurrentHashMap<>();
 
     public Database(ConnectionInfo ci, String cipher) {
         String name = ci.getName();
@@ -764,12 +761,10 @@ public class Database implements DataHandler {
             MetaRecord rec = new MetaRecord(cursor.get());
             objectIds.set(rec.getId());
             records.add(rec);
-            //System.out.println("Loaded:"+rec.toString());
         }
         Collections.sort(records);
         synchronized (systemSession) {
             for (MetaRecord rec : records) {
-                //System.out.println("Executing:"+rec.toString());
                 rec.execute(this, systemSession, eventListener);
             }
         }
@@ -938,33 +933,6 @@ public class Database implements DataHandler {
         boolean wasLocked = meta.lock(session, true, true);
         return wasLocked;
     }
-    public void traceLock(){
-        if(metaLockDebuggingStack.get()!=null && metaLockDebugging.get() !=null){
-            System.out.println("traceLock: Meta locked by sessionId="+metaLockDebugging.get().getId());
-            metaLockDebuggingStack.get().printStackTrace();
-        }
-    }
-
-    /**
-     * Lock the metadata table for updates - but don't wait if it's already locked...
-     *
-     * @param session the session
-     * @return whether it was already locked before by this session - or null if locked by other session
-     */
-    public Boolean lockMetaNoWait(Session session) {
-        // this method can not be synchronized on the database object,
-        // as unlocking is also synchronized on the database object -
-        // so if locking starts just before unlocking, locking could
-        // never be successful
-        if (meta == null) {
-            return true;
-        }
-        if(meta.isLockedExclusively() && ! meta.isLockedExclusivelyBy(session)){
-            return null;
-        }
-        boolean wasLocked = meta.lock(session, true, true);
-        return wasLocked;
-    }    
     
     /**
      * Unlock the metadata table.