Fixed recursive and non-recursive persistent views with CTE's

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

@@ -1138,7 +1138,7 @@ public class Parser {
 
             String[] querySQLOutput = new String[]{null};
             List<Column> columnTemplateList = createQueryColumnTemplateList(null, command.getQuery(), querySQLOutput);
-            TableView temporarySourceTableView = createTemporarySessionView(
+            TableView temporarySourceTableView = createCTEView(
                     command.getQueryAlias(), querySQLOutput[0],
                     columnTemplateList, false/* no recursion */,
                     false/* do not add to session */,
@@ -5130,12 +5130,12 @@ public class Parser {
         List<TableView> viewsCreated = new ArrayList<>();
         readIf("RECURSIVE");
         
-        // this WITH statement might not be temporary - allow optional keyword to tell us that
-        // this keyword is a work in progress feature and will not be documented
+        // this WITH statement might not be a temporary view - allow optional keyword to tell us that
+        // this keyword. This is a work in progress feature and will not be documented
         boolean isPersistent = readIf("PERSISTENT");
         
-        // this WITH statement might not be temporary - it may part of a persistent view
-        // as in CREATE VIEW abc AS WITH - this auto detects that condition
+        // this WITH statement is not be temporary - it is part of a persistent view
+        // as in CREATE VIEW abc AS WITH my_cte - this auto detects that condition
         if(session.isParsingView()){
             isPersistent = true;
         }
@@ -5145,10 +5145,11 @@ public class Parser {
         } while (readIf(","));
 
         Prepared p = null;
+        Collections.reverse(viewsCreated);
 
         if(isToken("SELECT")) {
             Query query = parseSelectUnion();
-            query.setPrepareAlways(true);
+            query.setPrepareAlways(!isPersistent);
             query.setNeverLazy(true);
             p = query;
         }
@@ -5185,43 +5186,56 @@ public class Parser {
         // clean up temporary views starting with last to first (in case of
         // dependencies) - but only if they are not persistent
         if(!isPersistent){
-            Collections.reverse(viewsCreated);
             p.setCteCleanups(viewsCreated);
         }
         return p;
     }
 
+    @SuppressWarnings("resource")// Eclipse thinks targetSession needs releasing
     private TableView parseSingleCommonTableExpression(boolean isPersistent) {
-        Session targetSession =  session; //isPersistent ? database.getSystemSession() :
-        String tempViewName = readIdentifierWithSchema();
+        Session targetSession =  isPersistent ? database.getSystemSession() : session;
+        String cteViewName = readIdentifierWithSchema();
         Schema schema = getSchema();
         Table recursiveTable=null;
         ArrayList<Column> columns = New.arrayList();
         String[] cols = null;
+        Database db = session.getDatabase();
 
         // column names are now optional - they can be inferred from the named
-        // query if not supplied
+        // 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
+                // we don't really know the type of the column, so UNKNOWN will
                 // have to do
                 columns.add(new Column(c, Value.STRING));
             }
         }
         
-        Table oldViewFound = targetSession.findLocalTempTable(tempViewName);
+        Table oldViewFound = null;
+        if(isPersistent){
+            oldViewFound = getSchema().findTableOrView(session, cteViewName);
+        }
+        else{
+            oldViewFound = targetSession.findLocalTempTable(cteViewName);
+        }
         if (oldViewFound != null && !isPersistent) {
             if (!(oldViewFound instanceof TableView)) {
                 throw DbException.get(ErrorCode.TABLE_OR_VIEW_ALREADY_EXISTS_1,
-                        tempViewName);
+                        cteViewName);
             }
             TableView tv = (TableView) oldViewFound;
             if (!tv.isTableExpression()) {
                 throw DbException.get(ErrorCode.TABLE_OR_VIEW_ALREADY_EXISTS_1,
-                        tempViewName);
+                        cteViewName);
             }
+            if(isPersistent){
+                oldViewFound.lock(session, true, true);
+                session.getDatabase().removeSchemaObject(session, oldViewFound);                
+                
+            }else{
                 targetSession.removeLocalTempTable(oldViewFound);
+            }
             oldViewFound=null;
         }
         // this table is created as a work around because recursive
@@ -5233,15 +5247,24 @@ public class Parser {
             CreateTableData recursiveTableData = new CreateTableData();
             recursiveTableData.id = database.allocateObjectId();
             recursiveTableData.columns = columns;
-            recursiveTableData.tableName = tempViewName;
+            recursiveTableData.tableName = cteViewName;
             recursiveTableData.temporary = !isPersistent;
             recursiveTableData.persistData = true;
             recursiveTableData.persistIndexes = isPersistent;
             recursiveTableData.create = true;
             recursiveTableData.session = targetSession;
+            // this gets a meta table lock that is not released
             recursiveTable = schema.createTable(recursiveTableData);
+            if(isPersistent){
+                // this unlock is to prevent beed from schema.createTable()
+                database.unlockMeta(targetSession);
+                synchronized (targetSession) {
+                    db.addSchemaObject(session, recursiveTable);
+                }
+            }else{
                 targetSession.addLocalTempTable(recursiveTable);
             }
+        }
         List<Column> columnTemplateList;
         String[] querySQLOutput = new String[]{null};
         try {
@@ -5253,14 +5276,20 @@ public class Parser {
 
         } finally {
             if(recursiveTable!=null){
+                if(isPersistent){
+                    recursiveTable.lock(session, true, true);
+                    session.getDatabase().removeSchemaObject(session, recursiveTable);
+                    
+                }else{
                     targetSession.removeLocalTempTable(recursiveTable);
                 }
             }
+        }
         // If it's persistent, a CTE and a TableView - return existing one, otherwise create new...
         if(oldViewFound!=null && isPersistent && oldViewFound instanceof TableView && oldViewFound.isTableExpression()){
             return (TableView) oldViewFound;
         }
-        TableView view = createTemporarySessionView(tempViewName,
+        TableView view = createCTEView(cteViewName,
                 querySQLOutput[0], columnTemplateList,
                 true/* allowRecursiveQueryDetection */, true/* add to session */, isPersistent);
         
@@ -5283,7 +5312,7 @@ public class Parser {
             Query theQuery, String[] querySQLOutput) {
         List<Column> columnTemplateList = new ArrayList<>();
         theQuery.prepare();
-        // array of length 1 to receive extra 'output' field in addition to
+        // array of length 1 is to receive extra 'output' field in addition to
         // return value
         querySQLOutput[0] = StringUtils.cache(theQuery.getPlanSQL());
         ColumnNamer columnNamer = new ColumnNamer(theQuery.getSession());
@@ -5300,31 +5329,47 @@ public class Parser {
         return columnTemplateList;
     }
 
-    private TableView createTemporarySessionView(String tempViewName,  String querySQL,
+    private TableView createCTEView(String cteViewName,  String querySQL,
             List<Column> columnTemplateList, boolean allowRecursiveQueryDetection, 
             boolean addViewToSession, boolean isPersistent) {
-        Session targetSession = session; //isPersistent ? database.getSystemSession() :
+        Session targetSession = /*isPersistent ? database.getSystemSession() :*/session;
+        Database db = session.getDatabase();
         Schema schema = getSchemaWithDefault();
         int id = database.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.
-        TableView view = new TableView(schema, id, tempViewName, querySQL,
-                parameters, columnTemplateList.toArray(new Column[0]), targetSession,
-                allowRecursiveQueryDetection, false);
+        TableView view;
+        synchronized(targetSession){            
+            view = new TableView(schema, id, cteViewName, querySQL,
+                    parameters, columnTemplateArray, targetSession,
+                    allowRecursiveQueryDetection, false /* literalsChecked */);
             if (!view.isRecursiveQueryDetected() && allowRecursiveQueryDetection) {
-            targetSession.removeLocalTempTable(view);
-            view = new TableView(schema, id, tempViewName, querySQL, parameters,
-                    columnTemplateList.toArray(new Column[0]), targetSession,
-                    false/* recursive */, false);
+                if(isPersistent){
+                    db.addSchemaObject(session, view);
+                    view.lock(session, true, true);
+                    session.getDatabase().removeSchemaObject(session, view);                
+                }else{
+                    session.removeLocalTempTable(view);                    
+                }
+                view = new TableView(schema, id, cteViewName, querySQL, parameters,
+                        columnTemplateArray, targetSession,
+                        false/* assume recursive */, false /* literalsChecked */);
+            }
         }
         view.setTableExpression(true);
         view.setTemporary(!isPersistent);
         view.setHidden(true);
+        view.setOnCommitDrop(false);
         if(addViewToSession){
+            if(isPersistent){
+                db.addSchemaObject(session, view);
+            }
+            else{
                 targetSession.addLocalTempTable(view);
             }
-        view.setOnCommitDrop(false);
+        }
         return view;
     }
 

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

@@ -100,6 +100,7 @@ public class CreateTable extends SchemaCommand {
 
     @Override
     public int update() {
+        boolean metaLockAquired = false;
         if (!transactional) {
             session.commit(true);
         }
@@ -108,8 +109,10 @@ public class CreateTable extends SchemaCommand {
             data.persistIndexes = false;
         }
         boolean isSessionTemporary = data.temporary && !data.globalTemporary;
+        try {
             if (!isSessionTemporary) {
                 db.lockMeta(session);
+                metaLockAquired = true;
             }
             if (getSchema().resolveTableOrView(session, data.tableName) != null) {
                 if (ifNotExists) {
@@ -225,6 +228,13 @@ public class CreateTable extends SchemaCommand {
                 }
                 throw e;
             }
+        }
+        finally{
+
+            if (!isSessionTemporary && metaLockAquired) {
+                db.unlockMeta(session);
+            }
+        }
         return 0;
     }
 

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

@@ -7,7 +7,6 @@ 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/engine/Database.java

@@ -1843,6 +1843,7 @@ public class Database implements DataHandler {
         int type = obj.getType();
         if (type == DbObject.TABLE_OR_VIEW) {
             Table table = (Table) obj;
+            table.setBeingDropped(true);
             if (table.isTemporary() && !table.isGlobalTemporary()) {
                 session.removeLocalTempTable(table);
                 return;

h2/src/main/org/h2/schema/Schema.java

@@ -639,9 +639,14 @@ public class Schema extends DbObjectBase {
      * @return the created {@link Table} object
      */
     public Table createTable(CreateTableData data) {
+        Database acquiredMetaLockDatabase = null;
+        try{
             synchronized (database) {
                 if (!data.temporary || data.globalTemporary) {
                     database.lockMeta(data.session);
+                    
+                    // remember to unlock the meta lock before we leave this method
+                    acquiredMetaLockDatabase = database;
                 }
                 data.schema = this;
                 if (data.tableEngine == null) {
@@ -656,11 +661,24 @@ public class Schema extends DbObjectBase {
                     if (data.tableEngineParams == null) {
                         data.tableEngineParams = this.tableEngineParams;
                     }
+                    
+                    // the createTable method unlocks the meta - so turn off flag now
+                    acquiredMetaLockDatabase=null;
                     return database.getTableEngine(data.tableEngine).createTable(data);
                 }
+                // the RegularTable constructor unlocks the meta - so turn off flag now
+                acquiredMetaLockDatabase=null;
                 return new RegularTable(data);
             }
         }
+        finally{
+            if(acquiredMetaLockDatabase!=null && data.session!=null){
+                if(acquiredMetaLockDatabase.isSysTableLockedBy(data.session)){
+                    acquiredMetaLockDatabase.unlockMeta(data.session);
+                }
+            }
+        }
+    }
 
     public TableSynonym createSynonym(CreateSynonymData data) {
         synchronized (database) {

h2/src/main/org/h2/table/Table.java

@@ -77,12 +77,13 @@ public abstract class Table extends SchemaObjectBase {
     private ArrayList<TriggerObject> triggers;
     private ArrayList<Constraint> constraints;
     private ArrayList<Sequence> sequences;
-    private ArrayList<TableView> views;
+    private ArrayList<TableView> views; // remember which views are using this object 
     private ArrayList<TableSynonym> synonyms;
     private boolean checkForeignKeyConstraints = true;
     private boolean onCommitDrop, onCommitTruncate;
     private volatile Row nullRow;
     private boolean tableExpression;
+    private boolean isBeingDropped;
     
 
     public Table(Schema schema, int id, String name, boolean persistIndexes,
@@ -158,7 +159,6 @@ public abstract class Table extends SchemaObjectBase {
      * @param key the primary key
      * @return the row
      */
-    @SuppressWarnings("unused")
     public Row getRow(Session session, long key) {
         return null;
     }
@@ -193,7 +193,6 @@ public abstract class Table extends SchemaObjectBase {
      * @param operation the operation
      * @param row the row
      */
-    @SuppressWarnings("unused")
     public void commit(short operation, Row row) {
         // nothing to do
     }
@@ -231,7 +230,6 @@ public abstract class Table extends SchemaObjectBase {
      * @param allColumnsSet all columns
      * @return the scan index
      */
-    @SuppressWarnings("unused")
     public Index getScanIndex(Session session, int[] masks,
             TableFilter[] filters, int filter, SortOrder sortOrder,
             HashSet<Column> allColumnsSet) {
@@ -464,7 +462,6 @@ public abstract class Table extends SchemaObjectBase {
      * @param session the session
      * @return true if it is
      */
-    @SuppressWarnings("unused")
     public boolean isLockedExclusivelyBy(Session session) {
         return false;
     }
@@ -1168,7 +1165,6 @@ public abstract class Table extends SchemaObjectBase {
      * @return an object array with the sessions involved in the deadlock, or
      *         null
      */
-    @SuppressWarnings("unused")
     public ArrayList<Session> checkDeadlock(Session session, Session clash,
             Set<Session> visited) {
         return null;
@@ -1253,4 +1249,12 @@ public abstract class Table extends SchemaObjectBase {
         return tableExpression;
     }
     
+    public boolean isBeingDropped(){
+        return isBeingDropped;
+    }
+    
+    public void setBeingDropped(boolean isBeingDropped){
+        this.isBeingDropped = isBeingDropped;
+    }
+
 }

h2/src/main/org/h2/table/TableView.java

@@ -58,6 +58,7 @@ public class TableView extends Table {
     private Query topQuery;
     private ResultInterface recursiveResult;
     private boolean isRecursiveQueryDetected;
+    private Session session;
 
     public TableView(Schema schema, int id, String name, String querySQL,
             ArrayList<Parameter> params, Column[] columnTemplates, Session session,
@@ -98,6 +99,7 @@ public class TableView extends Table {
         this.columnTemplates = columnTemplates;
         this.recursive = recursive;
         this.isRecursiveQueryDetected = false;
+        this.session = session;
         index = new ViewIndex(this, querySQL, params, recursive);
         initColumnsAndTables(session, literalsChecked);
     }
@@ -157,13 +159,13 @@ public class TableView extends Table {
         Column[] cols;
         removeViewFromTables();
         try {
-            Query query = compileViewQuery(session, querySQL, literalsChecked);
-            this.querySQL = query.getPlanSQL();
-            tables = New.arrayList(query.getTables());
-            ArrayList<Expression> expressions = query.getExpressions();
+            Query compiledQuery = compileViewQuery(session, querySQL, literalsChecked);
+            this.querySQL = compiledQuery.getPlanSQL();
+            tables = New.arrayList(compiledQuery.getTables());
+            ArrayList<Expression> expressions = compiledQuery.getExpressions();
             ArrayList<Column> list = New.arrayList();
             ColumnNamer columnNamer= new ColumnNamer(session);                        
-            for (int i = 0, count = query.getColumnCount(); i < count; i++) {
+            for (int i = 0, count = compiledQuery.getColumnCount(); i < count; i++) {
                 Expression expr = expressions.get(i);
                 String name = null;
                 int type = Value.UNKNOWN;
@@ -205,7 +207,7 @@ public class TableView extends Table {
             cols = new Column[list.size()];
             list.toArray(cols);
             createException = null;
-            viewQuery = query;
+            viewQuery = compiledQuery;
         } catch (DbException e) {
             e.addSQL(getCreateSQL());
             createException = e;
@@ -694,4 +696,16 @@ public class TableView extends Table {
         return true;
     }
     
+    @Override
+    public void removeView(TableView view){
+        super.removeView(view);
+        // if this is a table expression and the last view to use it is
+        // being dropped - then remove itself from the schema
+        if(isTableExpression() && getViews()!=null && view.isBeingDropped()){
+            // check if any database objects are left using this view
+            if(getViews().size()==0){
+                session.getDatabase().removeSchemaObject(session,this);
+            }            
+        }
+    }
 }

h2/src/test/org/h2/test/db/TestGeneralCommonTableQueries.java

@@ -44,6 +44,7 @@ public class TestGeneralCommonTableQueries extends TestBase {
         testNestedSQL();
         testRecursiveTable();
         testRecursiveTableInCreateView();
+        testNonRecursiveTableInCreateView();
     }
 
     private void testSimpleSelect() throws Exception {
@@ -608,4 +609,41 @@ public class TestGeneralCommonTableQueries extends TestBase {
         testRepeatedQueryWithSetup(maxRetries, expectedRowData, expectedColumnNames, expectedNumbeOfRows, SETUP_SQL,
                 WITH_QUERY);
     }
+    private void testNonRecursiveTableInCreateView() throws Exception {    
+        String SETUP_SQL = ""
+                +"DROP VIEW IF EXISTS v_my_nr_tree;                                                            \n"
+                +"DROP TABLE IF EXISTS my_table;                                                               \n"
+                +"CREATE TABLE my_table (                                                                      \n"
+                +" id INTEGER,                                                                                 \n"
+                +" parent_fk INTEGER                                                                           \n"
+                +");                                                                                           \n"
+                +"                                                                                             \n"
+                +"INSERT INTO my_table ( id, parent_fk) VALUES ( 1, NULL );                                    \n"
+                +"INSERT INTO my_table ( id, parent_fk) VALUES ( 11, 1 );                                      \n"
+                +"INSERT INTO my_table ( id, parent_fk) VALUES ( 111, 11 );                                    \n"
+                +"INSERT INTO my_table ( id, parent_fk) VALUES ( 12, 1 );                                      \n"
+                +"INSERT INTO my_table ( id, parent_fk) VALUES ( 121, 12 );                                    \n"
+                +"                                                                                             \n"
+                +"CREATE OR REPLACE VIEW v_my_nr_tree AS                                                       \n"
+                +"WITH tree_cte_nr (sub_tree_root_id, tree_level, parent_fk, child_fk) AS (                    \n"
+                +"    SELECT mt.ID AS sub_tree_root_id, CAST(0 AS INT) AS tree_level, mt.parent_fk, mt.id      \n"
+                +"      FROM my_table mt                                                                       \n"
+                +")                                                                                            \n"
+                +"SELECT sub_tree_root_id, tree_level, parent_fk, child_fk FROM tree_cte_nr;                   \n"
+                ;
+        
+        String WITH_QUERY = "SELECT * FROM v_my_nr_tree";
+        int maxRetries = 4;
+        String[] expectedRowData =new String[]{
+                "|1|0|null|1",
+                "|11|0|1|11",
+                "|111|0|11|111",
+                "|12|0|1|12",
+                "|121|0|12|121",
+                };
+        String[] expectedColumnNames =new String[]{"SUB_TREE_ROOT_ID","TREE_LEVEL","PARENT_FK","CHILD_FK"};
+        int expectedNumbeOfRows = 5;
+        testRepeatedQueryWithSetup(maxRetries, expectedRowData, expectedColumnNames, expectedNumbeOfRows, SETUP_SQL,
+                WITH_QUERY);
+    }    
 }