Merge pull request #1121 from katzyn/hash

Add some protection to ValueHashMap against hashes with the same less significant bits

h2/src/main/org/h2/util/ValueHashMap.java

@@ -54,7 +54,12 @@ public class ValueHashMap<V> extends HashBase {
     }
 
     private int getIndex(Value key) {
-        return key.hashCode() & mask;
+        int h = key.hashCode();
+        /*
+         * Add some protection against hashes with the same less significant bits
+         * (ValueDouble with integer values, for example).
+         */
+        return (h ^ h >>> 16) & mask;
     }
 
     /**

h2/src/main/org/h2/value/ValueDouble.java

@@ -133,8 +133,12 @@ public class ValueDouble extends Value {
 
     @Override
     public int hashCode() {
-        long hash = Double.doubleToLongBits(value);
-        return (int) (hash ^ (hash >> 32));
+        /*
+         * NaNs are normalized in get() method, so it's safe to use
+         * doubleToRawLongBits() instead of doubleToLongBits() here.
+         */
+        long hash = Double.doubleToRawLongBits(value);
+        return (int) (hash ^ (hash >>> 32));
     }
 
     @Override

h2/src/main/org/h2/value/ValueFloat.java

@@ -34,6 +34,7 @@ public class ValueFloat extends Value {
 
     private static final ValueFloat ZERO = new ValueFloat(0.0F);
     private static final ValueFloat ONE = new ValueFloat(1.0F);
+    private static final ValueFloat NAN = new ValueFloat(Float.NaN);
 
     private final float value;
 
@@ -88,7 +89,7 @@ public class ValueFloat extends Value {
             return "POWER(0, -1)";
         } else if (value == Float.NEGATIVE_INFINITY) {
             return "(-POWER(0, -1))";
-        } else if (Double.isNaN(value)) {
+        } else if (Float.isNaN(value)) {
             // NaN
             return "SQRT(-1)";
         }
@@ -133,8 +134,11 @@ public class ValueFloat extends Value {
 
     @Override
     public int hashCode() {
-        long hash = Float.floatToIntBits(value);
-        return (int) (hash ^ (hash >> 32));
+        /*
+         * NaNs are normalized in get() method, so it's safe to use
+         * floatToRawIntBits() instead of floatToIntBits() here.
+         */
+        return Float.floatToRawIntBits(value);
     }
 
     @Override
@@ -160,6 +164,8 @@ public class ValueFloat extends Value {
         } else if (d == 0.0F) {
             // -0.0 == 0.0, and we want to return 0.0 for both
             return ZERO;
+        } else if (Float.isNaN(d)) {
+            return NAN;
         }
         return (ValueFloat) Value.cache(new ValueFloat(d));
     }

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

@@ -10,6 +10,10 @@ import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.sql.Statement;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+
 import org.h2.test.TestBase;
 
 /**
@@ -29,16 +33,25 @@ public abstract class AbstractBaseForCommonTableExpressions extends TestBase {
      * @param closeAndReopenDatabaseConnectionOnIteration whether the connection
      *            should be re-opened each time
      * @param expectedColumnTypes the expected datatypes of the result
+     * @param anyOrder whether any order of rows should be allowed.
+     *                 If {@code true}, this method may sort expectedRowData.
      */
     void testRepeatedQueryWithSetup(int maxRetries, String[] expectedRowData, String[] expectedColumnNames,
             int expectedNumberOfRows, String setupSQL, String withQuery,
-            int closeAndReopenDatabaseConnectionOnIteration, String[] expectedColumnTypes) throws SQLException {
+            int closeAndReopenDatabaseConnectionOnIteration, String[] expectedColumnTypes,
+            boolean anyOrder) throws SQLException {
 
         deleteDb("commonTableExpressionQueries");
         Connection conn = getConnection("commonTableExpressionQueries");
         PreparedStatement prep;
         ResultSet rs;
 
+        if (anyOrder) {
+            Arrays.sort(expectedRowData);
+        }
+        ArrayList<String> rowData = new ArrayList<>();
+        StringBuilder buf = new StringBuilder();
+
         for (int queryRunTries = 1; queryRunTries <= maxRetries; queryRunTries++) {
 
             Statement stat = conn.createStatement();
@@ -65,17 +78,18 @@ public abstract class AbstractBaseForCommonTableExpressions extends TestBase {
                         expectedColumnTypes[columnIndex - 1], rs.getMetaData().getColumnTypeName(columnIndex));
             }
 
-            int rowNdx = 0;
+            rowData.clear();
             while (rs.next()) {
-                StringBuffer buf = new StringBuffer();
+                buf.setLength(0);
                 for (int columnIndex = 1; columnIndex <= rs.getMetaData().getColumnCount(); columnIndex++) {
-                    buf.append("|" + rs.getString(columnIndex));
+                    buf.append('|').append(rs.getString(columnIndex));
                 }
-                assertEquals(expectedRowData[rowNdx], buf.toString());
-                rowNdx++;
+                rowData.add(buf.toString());
             }
-
-            assertEquals(expectedNumberOfRows, rowNdx);
+            if (anyOrder) {
+                Collections.sort(rowData);
+            }
+            assertEquals(expectedRowData, rowData.toArray(new String[0]));
 
             rs.close();
             prep.close();

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

@@ -492,7 +492,7 @@ public class TestGeneralCommonTableQueries extends AbstractBaseForCommonTableExp
         int expectedNumberOfRows = expectedRowData.length;
 
         testRepeatedQueryWithSetup(maxRetries, expectedRowData, expectedColumnNames, expectedNumberOfRows, setupSQL,
-                withQuery, maxRetries - 1, expectedColumnTypes);
+                withQuery, maxRetries - 1, expectedColumnTypes, false);
 
     }
 
@@ -512,7 +512,7 @@ public class TestGeneralCommonTableQueries extends AbstractBaseForCommonTableExp
         int expectedNumberOfRows = expectedRowData.length;
 
         testRepeatedQueryWithSetup(maxRetries, expectedRowData, expectedColumnNames, expectedNumberOfRows, setupSQL,
-                withQuery, maxRetries - 1, expectedColumnTypes);
+                withQuery, maxRetries - 1, expectedColumnTypes, false);
 
     }
 
@@ -549,7 +549,7 @@ public class TestGeneralCommonTableQueries extends AbstractBaseForCommonTableExp
             int expectedNumberOfRows = expectedRowData.length;
 
             testRepeatedQueryWithSetup(maxRetries, expectedRowData, expectedColumnNames, expectedNumberOfRows,
-                    setupSQL, withQuery, maxRetries - 1, expectedColumnTypes);
+                    setupSQL, withQuery, maxRetries - 1, expectedColumnTypes, false);
         } finally {
             config = backupConfig;
         }
@@ -576,6 +576,6 @@ public class TestGeneralCommonTableQueries extends AbstractBaseForCommonTableExp
         int expectedNumberOfRows = expectedRowData.length;
 
         testRepeatedQueryWithSetup(maxRetries, expectedRowData, expectedColumnNames, expectedNumberOfRows, setupSQL,
-                withQuery, maxRetries - 1, expectedColumnTypes);
+                withQuery, maxRetries - 1, expectedColumnTypes, false);
     }
 }

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

@@ -98,7 +98,7 @@ public class TestPersistentCommonTableExpressions extends AbstractBaseForCommonT
         int expectedNumberOfRows = expectedRowData.length;
 
         testRepeatedQueryWithSetup(maxRetries, expectedRowData, expectedColumnNames, expectedNumberOfRows, setupSQL,
-                withQuery, maxRetries - 1, expectedColumnTypes);
+                withQuery, maxRetries - 1, expectedColumnTypes, true);
 
     }
 
@@ -147,7 +147,7 @@ public class TestPersistentCommonTableExpressions extends AbstractBaseForCommonT
         String[] expectedColumnTypes = new String[]{"INTEGER", "INTEGER", "INTEGER", "INTEGER"};
         int expectedNumberOfRows = 11;
         testRepeatedQueryWithSetup(maxRetries, expectedRowData, expectedColumnNames, expectedNumberOfRows, setupSQL,
-                withQuery, maxRetries - 1, expectedColumnTypes);
+                withQuery, maxRetries - 1, expectedColumnTypes, false);
     }
 
     private void testPersistentNonRecursiveTableInCreateView() throws Exception {
@@ -186,7 +186,7 @@ public class TestPersistentCommonTableExpressions extends AbstractBaseForCommonT
         String[] expectedColumnTypes = new String[]{"INTEGER", "INTEGER", "INTEGER", "INTEGER"};
         int expectedNumberOfRows = 5;
         testRepeatedQueryWithSetup(maxRetries, expectedRowData, expectedColumnNames, expectedNumberOfRows, setupSQL,
-                withQuery, maxRetries - 1, expectedColumnTypes);
+                withQuery, maxRetries - 1, expectedColumnTypes, false);
     }
 
     private void testPersistentNonRecursiveTableInCreateViewDropAllObjects() throws Exception {
@@ -224,7 +224,7 @@ public class TestPersistentCommonTableExpressions extends AbstractBaseForCommonT
         String[] expectedColumnTypes = new String[]{"INTEGER", "INTEGER", "INTEGER", "INTEGER"};
         int expectedNumberOfRows = 5;
         testRepeatedQueryWithSetup(maxRetries, expectedRowData, expectedColumnNames, expectedNumberOfRows, setupSQL,
-                withQuery, maxRetries - 1, expectedColumnTypes);
+                withQuery, maxRetries - 1, expectedColumnTypes, false);
     }
 
     private void testPersistentRecursiveTableInCreateViewDropAllObjects() throws Exception {
@@ -271,6 +271,6 @@ public class TestPersistentCommonTableExpressions extends AbstractBaseForCommonT
         String[] expectedColumnTypes = new String[]{"INTEGER", "INTEGER", "INTEGER", "INTEGER"};
         int expectedNumberOfRows = 11;
         testRepeatedQueryWithSetup(maxRetries, expectedRowData, expectedColumnNames, expectedNumberOfRows, setupSQL,
-                withQuery, maxRetries - 1, expectedColumnTypes);
+                withQuery, maxRetries - 1, expectedColumnTypes, false);
     }
 }

h2/src/test/org/h2/test/scripts/testScript.sql

@@ -6312,6 +6312,8 @@ SELECT 'abc', 'Papa Joe''s', CAST(-1 AS SMALLINT), CAST(2 AS BIGINT), CAST(0 AS
 > abc   Papa Joe's    -1 2 0.0 0a0f    125 TRUE FALSE
 > rows: 1
 
+-- ' This apostrophe is here to fix syntax highlighting in the text editors.
+
 SELECT CAST('abcd' AS VARCHAR(255)), CAST('ef_gh' AS VARCHAR(3));
 > 'abcd' 'ef_'
 > ------ -----
@@ -6885,14 +6887,14 @@ INSERT INTO TEST VALUES(?, ?, ?);
 };
 > update count: 9
 
-SELECT IFNULL(NAME, '') || ': ' || GROUP_CONCAT(VALUE ORDER BY NAME, VALUE DESC SEPARATOR ', ') FROM TEST GROUP BY NAME;
+SELECT IFNULL(NAME, '') || ': ' || GROUP_CONCAT(VALUE ORDER BY NAME, VALUE DESC SEPARATOR ', ') FROM TEST GROUP BY NAME ORDER BY 1;
 > (IFNULL(NAME, '') || ': ') || GROUP_CONCAT(VALUE ORDER BY NAME, VALUE DESC SEPARATOR ', ')
 > ------------------------------------------------------------------------------------------
+> : 3.10, -10.00
 > Apples: 1.50, 1.20, 1.10
-> Oranges: 2.05, 1.80
 > Bananas: 2.50
 > Cherries: 5.10
-> : 3.10, -10.00
+> Oranges: 2.05, 1.80
 > rows (ordered): 5
 
 SELECT GROUP_CONCAT(ID ORDER BY ID) FROM TEST;