Merge pull request #1707 from katzyn/result

Fix sort order and ENUM data type in external results

h2/src/docsrc/html/changelog.html

@@ -21,6 +21,8 @@ Change Log
 
 <h2>Next Version (unreleased)</h2>
 <ul>
+<li>PR #1707: Fix sort order and ENUM data type in external results
+</li>
 <li>PR #1706: Add hypothetical set functions
 </li>
 <li>PR #1705: Fix GROUP_CONCAT with variable separator

h2/src/main/org/h2/mvstore/db/MVPlainTempResult.java

@@ -59,8 +59,8 @@ class MVPlainTempResult extends MVTempResult {
      *            count of visible columns
      */
     MVPlainTempResult(Database database, Expression[] expressions, int visibleColumnCount) {
-        super(database, expressions.length, visibleColumnCount);
-        ValueDataType valueType = new ValueDataType(database, new int[columnCount]);
+        super(database, expressions, visibleColumnCount);
+        ValueDataType valueType = new ValueDataType(database, new int[expressions.length]);
         Builder<Long, ValueRow> builder = new MVMap.Builder<Long, ValueRow>()
                                                 .valueType(valueType).singleWriter();
         map = store.openMap("tmp", builder);
@@ -99,7 +99,11 @@ class MVPlainTempResult extends MVTempResult {
             return null;
         }
         cursor.next();
-        return cursor.getValue().getList();
+        Value[] currentRow = cursor.getValue().getList();
+        if (hasEnum) {
+            fixEnum(currentRow);
+        }
+        return currentRow;
     }
 
     @Override

h2/src/main/org/h2/mvstore/db/MVSortedTempResult.java

@@ -109,10 +109,10 @@ class MVSortedTempResult extends MVTempResult {
      */
     MVSortedTempResult(Database database, Expression[] expressions, boolean distinct, int[] distinctIndexes,
             int visibleColumnCount, SortOrder sort) {
-        super(database, expressions.length, visibleColumnCount);
+        super(database, expressions, visibleColumnCount);
         this.distinct = distinct;
         this.distinctIndexes = distinctIndexes;
-        int length = columnCount;
+        int length = expressions.length;
         int[] sortTypes = new int[length];
         int[] indexes;
         if (sort != null) {
@@ -192,7 +192,7 @@ class MVSortedTempResult extends MVTempResult {
                 if (index.putIfAbsent(distinctRow, true) != null) {
                     return rowCount;
                 }
-            } else if (columnCount != visibleColumnCount) {
+            } else if (expressions.length != visibleColumnCount) {
                 ValueRow distinctRow = ValueRow.get(Arrays.copyOf(values, visibleColumnCount));
                 if (index.putIfAbsent(distinctRow, true) != null) {
                     return rowCount;
@@ -221,7 +221,7 @@ class MVSortedTempResult extends MVTempResult {
             return parent.contains(values);
         }
         assert distinct;
-        if (columnCount != visibleColumnCount) {
+        if (expressions.length != visibleColumnCount) {
             return index.containsKey(ValueRow.get(values));
         }
         return map.containsKey(getKey(values));
@@ -297,6 +297,9 @@ class MVSortedTempResult extends MVTempResult {
         }
         // Read the next row
         current = getValue(cursor.next().getList());
+        if (hasEnum) {
+            fixEnum(current);
+        }
         /*
          * If valueCount is greater than 1 that is possible for non-distinct results the
          * following invocations of next() will use this.current and this.valueCount.
@@ -308,7 +311,7 @@ class MVSortedTempResult extends MVTempResult {
     @Override
     public int removeRow(Value[] values) {
         assert parent == null && distinct;
-        if (columnCount != visibleColumnCount) {
+        if (expressions.length != visibleColumnCount) {
             throw DbException.getUnsupportedException("removeRow()");
         }
         // If an entry was removed decrement the counter

h2/src/main/org/h2/mvstore/db/MVTempResult.java

@@ -19,6 +19,7 @@ import org.h2.result.ResultExternal;
 import org.h2.result.SortOrder;
 import org.h2.store.fs.FileUtils;
 import org.h2.util.TempFileDeleter;
+import org.h2.value.TypeInfo;
 import org.h2.value.Value;
 
 /**
@@ -87,15 +88,17 @@ public abstract class MVTempResult implements ResultExternal {
     final MVStore store;
 
     /**
-     * Count of columns.
+     * Column expressions.
      */
-    final int columnCount;
+    final Expression[] expressions;
 
     /**
      * Count of visible columns.
      */
     final int visibleColumnCount;
 
+    final boolean hasEnum;
+
     /**
      * Count of rows. Used only in a root results, copies always have 0 value.
      */
@@ -140,8 +143,9 @@ public abstract class MVTempResult implements ResultExternal {
     MVTempResult(MVTempResult parent) {
         this.parent = parent;
         this.store = parent.store;
-        this.columnCount = parent.columnCount;
+        this.expressions = parent.expressions;
         this.visibleColumnCount = parent.visibleColumnCount;
+        this.hasEnum = parent.hasEnum;
         this.tempFileDeleter = null;
         this.closeable = null;
         this.fileRef = null;
@@ -152,12 +156,12 @@ public abstract class MVTempResult implements ResultExternal {
      *
      * @param database
      *            database
-     * @param columnCount
-     *            count of columns
+     * @param expressions
+     *            column expressions
      * @param visibleColumnCount
      *            count of visible columns
      */
-    MVTempResult(Database database, int columnCount, int visibleColumnCount) {
+    MVTempResult(Database database, Expression[] expressions, int visibleColumnCount) {
         try {
             String fileName = FileUtils.createTempFile("h2tmp", Constants.SUFFIX_TEMP_FILE, true);
             Builder builder = new MVStore.Builder().fileName(fileName).cacheSize(0).autoCommitDisabled();
@@ -166,8 +170,16 @@ public abstract class MVTempResult implements ResultExternal {
                 builder.encryptionKey(MVTableEngine.decodePassword(key));
             }
             store = builder.open();
-            this.columnCount = columnCount;
+            this.expressions = expressions;
             this.visibleColumnCount = visibleColumnCount;
+            boolean hasEnum = false;
+            for (Expression e : expressions) {
+                if (e.getType().getValueType() == Value.ENUM) {
+                    hasEnum = true;
+                    break;
+                }
+            }
+            this.hasEnum = hasEnum;
             tempFileDeleter = database.getTempFileDeleter();
             closeable = new CloseImpl(store, fileName);
             fileRef = tempFileDeleter.addFile(closeable, this);
@@ -210,4 +222,13 @@ public abstract class MVTempResult implements ResultExternal {
         tempFileDeleter.deleteFile(fileRef, closeable);
     }
 
+    final void fixEnum(Value[] row) {
+        for (int i = 0, l = expressions.length; i < l; i++) {
+            TypeInfo type = expressions[i].getType();
+            if (type.getValueType() == Value.ENUM) {
+                row[i] = type.getExtTypeInfo().cast(row[i]);
+            }
+        }
+    }
+
 }

h2/src/main/org/h2/mvstore/db/ValueDataType.java

@@ -136,9 +136,9 @@ public class ValueDataType implements DataType {
         if (a == b) {
             return 0;
         }
-        if (a instanceof ValueArray && b instanceof ValueArray) {
-            Value[] ax = ((ValueArray) a).getList();
-            Value[] bx = ((ValueArray) b).getList();
+        if (a instanceof ValueCollectionBase && b instanceof ValueCollectionBase) {
+            Value[] ax = ((ValueCollectionBase) a).getList();
+            Value[] bx = ((ValueCollectionBase) b).getList();
             int al = ax.length;
             int bl = bx.length;
             int len = Math.min(al, bl);

h2/src/test/org/h2/test/TestAll.java

@@ -928,7 +928,6 @@ kill -9 `jps -l | grep "org.h2.test." | cut -d " " -f 1`
         addTest(new TestCluster());
         addTest(new TestFileLockSerialized());
         addTest(new TestFileLockProcess());
-        addTest(new TestFileSystem());
         addTest(new TestDefrag());
         addTest(new TestTools());
         addTest(new TestSampleApps());
@@ -973,6 +972,7 @@ kill -9 `jps -l | grep "org.h2.test." | cut -d " " -f 1`
         addTest(new TestDateIso8601());
         addTest(new TestDbException());
         addTest(new TestFile());
+        addTest(new TestFileSystem());
         addTest(new TestFtp());
         addTest(new TestGeometryUtils());
         addTest(new TestInterval());

h2/src/test/org/h2/test/unit/TestFileSystem.java

@@ -16,6 +16,7 @@ import java.nio.channels.FileChannel.MapMode;
 import java.nio.channels.FileLock;
 import java.nio.channels.NonWritableChannelException;
 import java.sql.Connection;
+import java.sql.DriverManager;
 import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.sql.Statement;
@@ -32,7 +33,6 @@ import org.h2.store.fs.FilePathEncrypt;
 import org.h2.store.fs.FilePathRec;
 import org.h2.store.fs.FileUtils;
 import org.h2.test.TestBase;
-import org.h2.test.TestDb;
 import org.h2.test.utils.AssertThrows;
 import org.h2.test.utils.FilePathDebug;
 import org.h2.tools.Backup;
@@ -43,7 +43,7 @@ import org.h2.util.Task;
 /**
  * Tests various file system.
  */
-public class TestFileSystem extends TestDb {
+public class TestFileSystem extends TestBase {
 
     /**
      * Run just this test.
@@ -251,7 +251,7 @@ public class TestFileSystem extends TestDb {
         FileUtils.deleteRecursive(dir, false);
         Connection conn;
         Statement stat;
-        conn = getConnection("jdbc:h2:split:18:"+dir+"/test");
+        conn = DriverManager.getConnection("jdbc:h2:split:18:"+dir+"/test");
         stat = conn.createStatement();
         stat.execute(
                 "create table test(id int primary key, name varchar) " +
@@ -260,7 +260,7 @@ public class TestFileSystem extends TestDb {
         conn.close();
         Backup.execute(dir + "/test.zip", dir, "", true);
         DeleteDbFiles.execute("split:" + dir, "test", true);
-        conn = getConnection(
+        conn = DriverManager.getConnection(
                 "jdbc:h2:split:zip:"+dir+"/test.zip!/test");
         conn.createStatement().execute("select * from test where id=1");
         conn.close();
@@ -269,22 +269,22 @@ public class TestFileSystem extends TestDb {
 
     private void testDatabaseInMemFileSys() throws SQLException {
         org.h2.Driver.load();
-        deleteDb("fsMem");
-        String url = "jdbc:h2:" + getBaseDir() + "/fsMem";
-        Connection conn = getConnection(url, "sa", "sa");
+        String dir = getBaseDir() + "/fsMem";
+        FileUtils.deleteRecursive(dir, false);
+        String url = "jdbc:h2:" + dir + "/fsMem";
+        Connection conn = DriverManager.getConnection(url, "sa", "sa");
         conn.createStatement().execute(
                 "CREATE TABLE TEST AS SELECT * FROM DUAL");
         conn.createStatement().execute(
                 "BACKUP TO '" + getBaseDir() + "/fsMem.zip'");
         conn.close();
-        org.h2.tools.Restore.main("-file", getBaseDir() + "/fsMem.zip", "-dir",
-                "memFS:");
-        conn = getConnection("jdbc:h2:memFS:fsMem", "sa", "sa");
+        org.h2.tools.Restore.main("-file", getBaseDir() + "/fsMem.zip", "-dir", "memFS:");
+        conn = DriverManager.getConnection("jdbc:h2:memFS:fsMem", "sa", "sa");
         ResultSet rs = conn.createStatement()
                 .executeQuery("SELECT * FROM TEST");
         rs.close();
         conn.close();
-        deleteDb("fsMem");
+        FileUtils.deleteRecursive(dir, false);
         FileUtils.delete(getBaseDir() + "/fsMem.zip");
         FileUtils.delete("memFS:fsMem.mv.db");
     }
@@ -297,8 +297,9 @@ public class TestFileSystem extends TestDb {
             return;
         }
         org.h2.Driver.load();
-        String url = "jdbc:h2:" + getBaseDir() + "/fsJar";
-        Connection conn = getConnection(url, "sa", "sa");
+        String dir = getBaseDir() + "/fsJar";
+        String url = "jdbc:h2:" + dir + "/fsJar";
+        Connection conn = DriverManager.getConnection(url, "sa", "sa");
         Statement stat = conn.createStatement();
         stat.execute("create table test(id int primary key, " +
                 "name varchar, b blob, c clob)");
@@ -310,12 +311,12 @@ public class TestFileSystem extends TestDb {
         byte[] b1 = rs.getBytes(3);
         String s1 = rs.getString(4);
         conn.close();
-        conn = getConnection(url, "sa", "sa");
+        conn = DriverManager.getConnection(url, "sa", "sa");
         stat = conn.createStatement();
         stat.execute("backup to '" + getBaseDir() + "/fsJar.zip'");
         conn.close();
 
-        deleteDb("fsJar");
+        FileUtils.deleteRecursive(dir, false);
         for (String f : FileUtils.newDirectoryStream(
                 "zip:" + getBaseDir() + "/fsJar.zip")) {
             assertFalse(FileUtils.isAbsolute(f));
@@ -334,7 +335,7 @@ public class TestFileSystem extends TestDb {
             testReadOnly(f);
         }
         String urlJar = "jdbc:h2:zip:" + getBaseDir() + "/fsJar.zip!/fsJar";
-        conn = getConnection(urlJar, "sa", "sa");
+        conn = DriverManager.getConnection(urlJar, "sa", "sa");
         stat = conn.createStatement();
         rs = stat.executeQuery("select * from test");
         rs.next();

h2/src/test/org/h2/test/unit/TestMVTempResult.java

@@ -11,6 +11,7 @@ import java.sql.DriverManager;
 import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.sql.Statement;
+import java.util.BitSet;
 
 import org.h2.test.TestBase;
 import org.h2.tools.DeleteDbFiles;
@@ -53,16 +54,25 @@ public class TestMVTempResult extends TestBase {
         DeleteDbFiles.execute(dir, name, true);
         try (Connection c = DriverManager.getConnection("jdbc:h2:" + dir + '/' + name)) {
             Statement s = c.createStatement();
-            try (ResultSet rs = s.executeQuery("SELECT X, RAND() R FROM SYSTEM_RANGE(1, " + ROWS + ") ORDER BY R")) {
-                for (int i = 1; i <= ROWS; i++) {
+            s.execute("CREATE TABLE TEST(I BIGINT, E ENUM('a', 'b'))" //
+                    + " AS SELECT X, 'a' FROM SYSTEM_RANGE(1, " + ROWS + ')');
+            try (ResultSet rs = s.executeQuery("SELECT I, E FROM TEST ORDER BY I DESC")) {
+                for (int i = ROWS; i > 0; i--) {
                     assertTrue(rs.next());
+                    assertEquals(i, rs.getLong(1));
+                    assertEquals("a", rs.getString(2));
                 }
+                assertFalse(rs.next());
             }
-            try (ResultSet rs = s.executeQuery("SELECT X, RAND() FROM SYSTEM_RANGE(1, " + ROWS + ')')) {
+            BitSet set = new BitSet(ROWS);
+            try (ResultSet rs = s.executeQuery("SELECT I, E FROM TEST")) {
                 for (int i = 1; i <= ROWS; i++) {
                     assertTrue(rs.next());
-                    assertEquals(i, rs.getLong(1));
+                    set.set((int) rs.getLong(1));
+                    assertEquals("a", rs.getString(2));
                 }
+                assertFalse(rs.next());
+                assertEquals(ROWS, set.cardinality());
             }
         }
         DeleteDbFiles.execute(dir, name, true);