Spatial index: a few bugs have been fixed (using spatial constraints in views,…

Spatial index: a few bugs have been fixed (using spatial constraints in views, transfering geometry objects over TCP/IP).

h2/src/docsrc/html/changelog.html

@@ -18,7 +18,9 @@ Change Log
 <h1>Change Log</h1>
 
 <h2>Next Version (unreleased)</h2>
-<ul><li>Issue 551: the datatype documentation was incorrect (found by Bernd Eckenfels).
+<ul><li>Spatial index: a few bugs have been fixed (using spatial constraints in views,
+    transfering geometry objects over TCP/IP).
+</li><li>Issue 551: the datatype documentation was incorrect (found by Bernd Eckenfels).
 </li><li>Issue 368: ON DUPLICATE KEY UPDATE did not work for multi-row inserts.
     Test case from Angus Macdonald.
 </li><li>OSGi: the package javax.tools is now imported (as an optional).

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

@@ -82,6 +82,11 @@ public class Constants {
      */
     public static final int TCP_PROTOCOL_VERSION_13 = 13;
 
+    /**
+     * The TCP protocol version number 14.
+     */
+    public static final int TCP_PROTOCOL_VERSION_14 = 14;
+
     /**
      * The major version of this database.
      */

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

@@ -103,7 +103,7 @@ public class SessionRemote extends SessionWithState implements DataHandler {
         trans.setSSL(ci.isSSL());
         trans.init();
         trans.writeInt(Constants.TCP_PROTOCOL_VERSION_6);
-        trans.writeInt(Constants.TCP_PROTOCOL_VERSION_13);
+        trans.writeInt(Constants.TCP_PROTOCOL_VERSION_14);
         trans.writeString(db);
         trans.writeString(ci.getOriginalURL());
         trans.writeString(ci.getUserName());
@@ -118,7 +118,7 @@ public class SessionRemote extends SessionWithState implements DataHandler {
             done(trans);
             clientVersion = trans.readInt();
             trans.setVersion(clientVersion);
-            if (clientVersion >= Constants.TCP_PROTOCOL_VERSION_13) {
+            if (clientVersion >= Constants.TCP_PROTOCOL_VERSION_14) {
                 if (ci.getFileEncryptionKey() != null) {
                     trans.writeBytes(ci.getFileEncryptionKey());
                 }

h2/src/main/org/h2/index/ViewIndex.java

@@ -35,7 +35,7 @@ import org.h2.value.Value;
  * This object represents a virtual index for a query.
  * Actually it only represents a prepared SELECT statement.
  */
-public class ViewIndex extends BaseIndex {
+public class ViewIndex extends BaseIndex implements SpatialIndex {
 
     private final TableView view;
     private final String querySQL;
@@ -144,6 +144,9 @@ public class ViewIndex extends BaseIndex {
                 if ((mask & IndexCondition.EQUALITY) != 0) {
                     Parameter param = new Parameter(nextParamIndex);
                     q.addGlobalCondition(param, idx, Comparison.EQUAL_NULL_SAFE);
+                } else if ((mask & IndexCondition.SPATIAL_INTERSECTS) != 0) {
+                    Parameter param = new Parameter(nextParamIndex);
+                    q.addGlobalCondition(param, idx, Comparison.SPATIAL_INTERSECTS);
                 } else {
                     if ((mask & IndexCondition.START) != 0) {
                         Parameter param = new Parameter(nextParamIndex);
@@ -168,6 +171,15 @@ public class ViewIndex extends BaseIndex {
 
     @Override
     public Cursor find(Session session, SearchRow first, SearchRow last) {
+        return find(session, first, last, null);
+    }
+    
+    @Override
+    public Cursor findByGeometry(TableFilter filter, SearchRow intersection) {
+        return find(filter.getSession(), null, null, intersection);
+    }
+
+    private Cursor find(Session session, SearchRow first, SearchRow last, SearchRow intersection) {
         if (recursive) {
             ResultInterface recResult = view.getRecursiveResult();
             if (recResult != null) {
@@ -226,6 +238,8 @@ public class ViewIndex extends BaseIndex {
             len = first.getColumnCount();
         } else if (last != null) {
             len = last.getColumnCount();
+        } else if (intersection != null) {
+            len = intersection.getColumnCount();
         } else {
             len = 0;
         }
@@ -239,9 +253,19 @@ public class ViewIndex extends BaseIndex {
                     setParameter(paramList, x, v);
                 }
             }
-            // for equality, only one parameter is used (first == last)
-            if (last != null && indexMasks[i] != IndexCondition.EQUALITY) {
-                Value v = last.getValue(i);
+            if (last != null) {
+                int mask = indexMasks[i];
+                // for equality, only one parameter is used (first == last)
+                if (mask != IndexCondition.EQUALITY) {
+                    Value v = last.getValue(i);
+                    if (v != null) {
+                        int x = idx++;
+                        setParameter(paramList, x, v);
+                    }
+                }
+            }
+            if (intersection != null) {
+                Value v = intersection.getValue(i);
                 if (v != null) {
                     int x = idx++;
                     setParameter(paramList, x, v);
@@ -292,21 +316,26 @@ public class ViewIndex extends BaseIndex {
             int idx = paramIndex.get(i);
             columnList.add(table.getColumn(idx));
             int mask = masks[idx];
-            if ((mask & IndexCondition.EQUALITY) == IndexCondition.EQUALITY) {
+            if ((mask & IndexCondition.EQUALITY) != 0) {
                 Parameter param = new Parameter(firstIndexParam + i);
                 q.addGlobalCondition(param, idx, Comparison.EQUAL_NULL_SAFE);
                 i++;
             }
-            if ((mask & IndexCondition.START) == IndexCondition.START) {
+            if ((mask & IndexCondition.START) != 0) {
                 Parameter param = new Parameter(firstIndexParam + i);
                 q.addGlobalCondition(param, idx, Comparison.BIGGER_EQUAL);
                 i++;
             }
-            if ((mask & IndexCondition.END) == IndexCondition.END) {
+            if ((mask & IndexCondition.END) != 0) {
                 Parameter param = new Parameter(firstIndexParam + i);
                 q.addGlobalCondition(param, idx, Comparison.SMALLER_EQUAL);
                 i++;
             }
+            if ((mask & IndexCondition.SPATIAL_INTERSECTS) != 0) {
+                Parameter param = new Parameter(firstIndexParam + i);
+                q.addGlobalCondition(param, idx, Comparison.SPATIAL_INTERSECTS);
+                i++;
+            }
         }
         columns = new Column[columnList.size()];
         columnList.toArray(columns);
@@ -321,13 +350,13 @@ public class ViewIndex extends BaseIndex {
                     continue;
                 }
                 if (type == 0) {
-                    if ((mask & IndexCondition.EQUALITY) != IndexCondition.EQUALITY) {
+                    if ((mask & IndexCondition.EQUALITY) == 0) {
                         // the first columns need to be equality conditions
                         continue;
                     }
                 } else {
-                    if ((mask & IndexCondition.EQUALITY) == IndexCondition.EQUALITY) {
-                        // then only range conditions
+                    if ((mask & IndexCondition.EQUALITY) != 0) {
+                        // after that only range conditions
                         continue;
                     }
                 }

h2/src/main/org/h2/server/TcpServerThread.java

@@ -83,13 +83,15 @@ public class TcpServerThread implements Runnable {
                 }
                 int minClientVersion = transfer.readInt();
                 if (minClientVersion < Constants.TCP_PROTOCOL_VERSION_6) {
-                    throw DbException.get(ErrorCode.DRIVER_VERSION_ERROR_2, "" + clientVersion, "" + Constants.TCP_PROTOCOL_VERSION_6);
-                } else if (minClientVersion > Constants.TCP_PROTOCOL_VERSION_13) {
-                    throw DbException.get(ErrorCode.DRIVER_VERSION_ERROR_2, "" + clientVersion, "" + Constants.TCP_PROTOCOL_VERSION_13);
+                    throw DbException.get(ErrorCode.DRIVER_VERSION_ERROR_2, 
+                            "" + clientVersion, "" + Constants.TCP_PROTOCOL_VERSION_6);
+                } else if (minClientVersion > Constants.TCP_PROTOCOL_VERSION_14) {
+                    throw DbException.get(ErrorCode.DRIVER_VERSION_ERROR_2, 
+                            "" + clientVersion, "" + Constants.TCP_PROTOCOL_VERSION_14);
                 }
                 int maxClientVersion = transfer.readInt();
-                if (maxClientVersion >= Constants.TCP_PROTOCOL_VERSION_13) {
-                    clientVersion = Constants.TCP_PROTOCOL_VERSION_13;
+                if (maxClientVersion >= Constants.TCP_PROTOCOL_VERSION_14) {
+                    clientVersion = Constants.TCP_PROTOCOL_VERSION_14;
                 } else {
                     clientVersion = minClientVersion;
                 }

h2/src/main/org/h2/store/FileLock.java

@@ -238,7 +238,7 @@ public class FileLock implements Runnable {
             transfer.setSocket(socket);
             transfer.init();
             transfer.writeInt(Constants.TCP_PROTOCOL_VERSION_6);
-            transfer.writeInt(Constants.TCP_PROTOCOL_VERSION_13);
+            transfer.writeInt(Constants.TCP_PROTOCOL_VERSION_14);
             transfer.writeString(null);
             transfer.writeString(null);
             transfer.writeString(id);

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

@@ -508,7 +508,11 @@ public class Transfer {
             break;
         }
         case Value.GEOMETRY:
-            writeString(v.getString());
+            if (version >= Constants.TCP_PROTOCOL_VERSION_14) {
+                writeBytes(v.getBytesNoCopy());
+            } else {
+                writeString(v.getString());
+            }
             break;
         default:
             throw DbException.get(ErrorCode.CONNECTION_BROKEN_1, "type=" + type);
@@ -675,6 +679,9 @@ public class Transfer {
             return ValueResultSet.get(rs);
         }
         case Value.GEOMETRY:
+            if (version >= Constants.TCP_PROTOCOL_VERSION_14) {
+                return ValueGeometry.get(readBytes());
+            }
             return ValueGeometry.get(readString());
         default:
             throw DbException.get(ErrorCode.CONNECTION_BROKEN_1, "type=" + type);

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

@@ -32,26 +32,34 @@ import com.vividsolutions.jts.io.WKTWriter;
  */
 public class ValueGeometry extends Value {
 
+    /**
+     * As conversion from/to WKB cost a significant amount of CPU cycles, WKB
+     * are kept in ValueGeometry instance.
+     * 
+     * We always calculate the WKB, because not all WKT values can be
+     * represented in WKB, but since we persist it in WKB format, it has to be
+     * valid in WKB
+     */
+    private final byte[] bytes;
+
+    private final int hashCode;
+    
     /**
      * The value. Converted from WKB only on request as conversion from/to WKB
-     * cost a significant amount of cpu cycles.
+     * cost a significant amount of CPU cycles.
      */
     private Geometry geometry;
 
     /**
-     * As conversion from/to WKB cost a significant amount of cpu cycles, WKB
-     * are kept in ValueGeometry instance
+     * Create a new geometry objects.
+     * 
+     * @param bytes the bytes (always known)
+     * @param geometry the geometry object (may be null)
      */
-    private byte[] bytes;
-
-    private int hashCode;
-
-    private ValueGeometry(Geometry geometry) {
-        this.geometry = geometry;
-    }
-
-    private ValueGeometry(byte[] bytes) {
+    private ValueGeometry(byte[] bytes, Geometry geometry) {
         this.bytes = bytes;
+        this.geometry = geometry;
+        this.hashCode = Arrays.hashCode(bytes);
     }
 
     /**
@@ -65,12 +73,23 @@ public class ValueGeometry extends Value {
     }
 
     private static ValueGeometry get(Geometry g) {
-        // not all WKT values can be represented in WKB, but since we persist it
-        // in WKB format, it has to be valid in WKB
-        toWKB(g);
-        return (ValueGeometry) Value.cache(new ValueGeometry(g));
+        byte[] bytes = convertToWKB(g);
+        return (ValueGeometry) Value.cache(new ValueGeometry(bytes, g));
     }
-
+    
+    private static byte[] convertToWKB(Geometry g) {
+        boolean includeSRID = g.getSRID() != 0;
+        int dimensionCount = getDimensionCount(g);
+        WKBWriter writer = new WKBWriter(dimensionCount, includeSRID);
+        return writer.write(g);        
+    }
+    
+    private static int getDimensionCount(Geometry geometry) {
+        ZVisitor finder = new ZVisitor();
+        geometry.apply(finder);
+        return finder.isFoundZ() ? 3 : 2;
+    }
+    
     /**
      * Get or create a geometry value for the given geometry.
      *
@@ -78,11 +97,12 @@ public class ValueGeometry extends Value {
      * @return the value
      */
     public static ValueGeometry get(String s) {
-        Geometry g = fromWKT(s);
-        // not all WKT values can be represented in WKB, but since we persist it
-        // in WKB format, it has to be valid in WKB
-        toWKB(g);
-        return (ValueGeometry) Value.cache(new ValueGeometry(g));
+        try {
+            Geometry g = new WKTReader().read(s);
+            return get(g);
+        } catch (ParseException ex) {
+            throw DbException.convert(ex);
+        }
     }
 
     /**
@@ -92,16 +112,20 @@ public class ValueGeometry extends Value {
      * @return the value
      */
     public static ValueGeometry get(byte[] bytes) {
-        return (ValueGeometry) Value.cache(new ValueGeometry(bytes));
+        return (ValueGeometry) Value.cache(new ValueGeometry(bytes, null));
     }
 
     public Geometry getGeometry() {
-        if (geometry == null && bytes != null) {
-            geometry = fromWKB(bytes);
+        if (geometry == null) {
+            try {
+                geometry = new WKBReader().read(bytes);
+            } catch (ParseException ex) {
+                throw DbException.convert(ex);
+            }
         }
         return geometry;
     }
-
+    
     /**
      * Test if this geometry envelope intersects with the other geometry
      * envelope.
@@ -154,7 +178,10 @@ public class ValueGeometry extends Value {
 
     @Override
     public String getSQL() {
-        return StringUtils.quoteStringSQL(toWKT()) + "'::Geometry";
+        // WKT does not hold Z or SRID with JTS 1.13
+        // As getSQL is used to export database, it should contains all object attributes
+        // Moreover using bytes is faster than converting WKB to Geometry then to WKT.
+        return "X'" + StringUtils.convertBytesToHex(getBytesNoCopy()) + "'::Geometry";
     }
 
     @Override
@@ -165,7 +192,7 @@ public class ValueGeometry extends Value {
 
     @Override
     public String getString() {
-        return toWKT();
+        return getWKT();
     }
 
     @Override
@@ -175,9 +202,6 @@ public class ValueGeometry extends Value {
 
     @Override
     public int hashCode() {
-        if (hashCode == 0) {
-            hashCode = Arrays.hashCode(toWKB());
-        }
         return hashCode;
     }
 
@@ -188,12 +212,12 @@ public class ValueGeometry extends Value {
 
     @Override
     public byte[] getBytes() {
-        return toWKB();
+        return getWKB();
     }
 
     @Override
     public byte[] getBytesNoCopy() {
-        return toWKB();
+        return getWKB();
     }
 
     @Override
@@ -203,81 +227,37 @@ public class ValueGeometry extends Value {
 
     @Override
     public int getDisplaySize() {
-        return toWKT().length();
+        return getWKT().length();
     }
 
     @Override
     public int getMemory() {
-        return toWKB().length * 20 + 24;
+        return getWKB().length * 20 + 24;
     }
 
     @Override
     public boolean equals(Object other) {
         // The JTS library only does half-way support for 3D coordinates, so
         // their equals method only checks the first two coordinates.
-        return other instanceof ValueGeometry && Arrays.equals(toWKB(), ((ValueGeometry) other).toWKB());
+        return other instanceof ValueGeometry && Arrays.equals(getWKB(), ((ValueGeometry) other).getWKB());
     }
 
     /**
-     * Convert the value to the Well-Known-Text format.
+     * Get the value in Well-Known-Text format.
      *
      * @return the well-known-text
      */
-    public String toWKT() {
+    public String getWKT() {
         return new WKTWriter().write(getGeometry());
     }
 
     /**
-     * Convert to Well-Known-Binary format.
+     * Get the value in Well-Known-Binary format.
      *
      * @return the well-known-binary
      */
-    public byte[] toWKB() {
-        if (bytes != null) {
-            return bytes;
-        }
-        return toWKB(getGeometry());
-    }
-
-    private static byte[] toWKB(Geometry geometry) {
-        int dimensionCount = getDimensionCount(geometry);
-        boolean includeSRID = geometry.getSRID() != 0;
-        WKBWriter writer = new WKBWriter(dimensionCount, includeSRID);
-        return writer.write(geometry);
-    }
-
-    private static int getDimensionCount(Geometry geometry) {
-        ZVisitor finder = new ZVisitor();
-        geometry.apply(finder);
-        return finder.isFoundZ() ? 3 : 2;
-    }
-
-    /**
-     * Convert a Well-Known-Text to a Geometry object.
-     *
-     * @param s the well-known-text
-     * @return the Geometry object
-     */
-    private static Geometry fromWKT(String s) {
-        try {
-            return new WKTReader().read(s);
-        } catch (ParseException ex) {
-            throw DbException.convert(ex);
-        }
-    }
-
-    /**
-     * Convert a Well-Known-Binary to a Geometry object.
-     *
-     * @param bytes the well-known-binary
-     * @return the Geometry object
-     */
-    private static Geometry fromWKB(byte[] bytes) {
-        try {
-            return new WKBReader().read(bytes);
-        } catch (ParseException ex) {
-            throw DbException.convert(ex);
-        }
+    public byte[] getWKB() {
+        return bytes;
     }
 
     @Override

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

@@ -727,7 +727,7 @@ kill -9 `jps -l | grep "org.h2.test." | cut -d " " -f 1`
 
         // synth
         new TestBtreeIndex().runTest(this);
-     ;   // new TestDiskFull().runTest(this);
+        new TestDiskFull().runTest(this);
         new TestCrashAPI().runTest(this);
         new TestFuzzOptimizations().runTest(this);
         new TestLimit().runTest(this);

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

@@ -81,6 +81,8 @@ public class TestSpatial extends TestBase {
         testTableFunctionGeometry();
         testHashCode();
         testAggregateWithGeometry();
+        testTableViewSpatialPredicate();
+        testValueGeometryScript();
     }
 
     private void testHashCode() {
@@ -704,4 +706,55 @@ public class TestSpatial extends TestBase {
         }
     }
 
+    private void testTableViewSpatialPredicate() throws SQLException {
+        deleteDb("spatial");
+        Connection conn = getConnection(url);
+        try {
+            Statement stat = conn.createStatement();
+            stat.execute("drop table if exists test");
+            stat.execute("drop view if exists test_view");
+            stat.execute("create table test(id int primary key, poly geometry)");
+            stat.execute("insert into test values(1, 'POLYGON ((1 1, 1 2, 2 2, 1 1))')");
+            stat.execute("insert into test values(2, 'POLYGON ((3 1, 3 2, 4 2, 3 1))')");
+            stat.execute("insert into test values(3, 'POLYGON ((1 3, 1 4, 2 4, 1 3))')");
+            stat.execute("create view test_view as select * from test");
+
+            //Check result with view
+            ResultSet rs;
+            rs = stat.executeQuery(
+                    "select * from test where poly && 'POINT (1.5 1.5)'::Geometry");
+            assertTrue(rs.next());
+            assertEquals(1, rs.getInt("id"));
+            assertFalse(rs.next());
+            
+            rs = stat.executeQuery(
+                    "select * from test_view where poly && 'POINT (1.5 1.5)'::Geometry");
+            assertTrue(rs.next());
+            assertEquals(1, rs.getInt("id"));
+            assertFalse(rs.next());
+            rs.close();
+        } finally {
+            // Close the database
+            conn.close();
+        }
+        deleteDb("spatial");
+    }
+
+    /**
+     * Check ValueGeometry conversion into SQL script
+     */
+    private void testValueGeometryScript() throws SQLException {
+        ValueGeometry valueGeometry = ValueGeometry.get("POINT(1 1 5)");
+        Connection conn = getConnection(url);
+        try {
+            ResultSet rs = conn.createStatement().executeQuery(
+                    "SELECT " + valueGeometry.getSQL());
+            assertTrue(rs.next());
+            Object obj = rs.getObject(1);
+            ValueGeometry g = ValueGeometry.getFromGeometry(obj);
+            assertTrue("got: " + g + " exp: " + valueGeometry, valueGeometry.equals(g));
+        } finally {
+            conn.close();
+        }
+    }
 }