neo4j-contrib / spatial

Neo4j Spatial is a library of utilities for Neo4j that faciliates the enabling of spatial operations on data. In particular you can add spatial indexes to already located data, and perform spatial operations on the data like searching for data within specified regions or within a specified distance of a point of interest. In addition classes are provided to expose the data to geotools and thereby to geotools enabled applications like geoserver and uDig.
http://neo4j-contrib.github.io/spatial
Other
778 stars 192 forks source link

cypher procedure call: `call spatial.removeLayer()` deletes associated data nodes #283

Open LeBlue opened 7 years ago

LeBlue commented 7 years ago

cypher procedure call call spatial.removeLayer() deletes associated data nodes

I created some nodes in the neo4j shell and added them to a point layer:

    call spatial.addPointLayerXY("someLayer", "longitude", "latitude");
    create (loc:Location {name: "somewhere", longitude: 50.0, latitude: 20.0});

    match (loc:Location) with loc as location 
    call spatial.addNode("someLayer", location) yield node return node;

After removing the layer all nodes are gone

    call spatial.removeLayer("someLayer");
    match (loc:Location) return loc;

Is this intended behavior?

I am using neo-community-3.0.6 and neo4j-spatial-0.23-neo4j-3.0.4-server-plugin

nw31304 commented 7 years ago

Amen, Brother.

Did you get anywhere with this? I've been monitoring this thread hoping to discover whether I'm using the spatial plugin correctly. After removing a layer and watching my entire database disappear I added a plugin procedure called "removeLayerOnly" that removes the layer but bypasses the removal of the geometry nodes.

There's a telling comment in the Layer.java JavaDoc for delete():

    /**
     * Delete the entire layer, including the index. The specific layer implementation will decide
     * if this method should delete also the geometry nodes indexed by this layer. Some
     * implementations have data that only has meaning within a layer, and so will be deleted.
     * Others are simply views onto other more complex data models and deleting the geometry nodes
     * might imply damage to the model. Keep this in mind when coding implementations of the Layer.
     */
    void delete(Listener monitor);

Incidentally, on another occasion I tried to delete a layer with several million indexed nodes and (fortunately) Neo4J took a OutOfMemory exception because it tried to delete them all in a single transaction.

neo4j> call spatial.procedures;
+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| name                              | signature                                                                                                                         |
+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| "spatial.addNode"                 | "spatial.addNode(layerName :: STRING?, node :: NODE?) :: (node :: NODE?)"                                                         |
| "spatial.importShapefileToLayer"  | "spatial.importShapefileToLayer(layerName :: STRING?, uri :: STRING?) :: (count :: INTEGER?)"                                     |
| "spatial.decodeGeometry"          | "spatial.decodeGeometry(layerName :: STRING?, node :: NODE?) :: (geometry :: ANY?)"                                               |
| "spatial.addPointLayerXY"         | "spatial.addPointLayerXY(name :: STRING?, xProperty :: STRING?, yProperty :: STRING?) :: (node :: NODE?)"                         |
| "spatial.asExternalGeometry"      | "spatial.asExternalGeometry(geometry :: ANY?) :: (geometry :: ANY?)"                                                              |
| "spatial.removeLayer"             | "spatial.removeLayer(name :: STRING?) :: VOID"                                                                                    |
| "spatial.importShapefile"         | "spatial.importShapefile(uri :: STRING?) :: (count :: INTEGER?)"                                                                  |
| "spatial.addNodes"                | "spatial.addNodes(layerName :: STRING?, nodes :: LIST? OF NODE?) :: (count :: INTEGER?)"                                          |
| "spatial.layer"                   | "spatial.layer(name :: STRING?) :: (node :: NODE?)"                                                                               |
| "spatial.intersects"              | "spatial.intersects(layerName :: STRING?, geometry :: ANY?) :: (node :: NODE?)"                                                   |
| "spatial.removeLayerOnly"         | "spatial.removeLayerOnly(name :: STRING?) :: VOID"                                                                                |
| "spatial.updateFromWKT"           | "spatial.updateFromWKT(layerName :: STRING?, geometry :: STRING?, geometryNodeId :: INTEGER?) :: (node :: NODE?)"                 |
| "spatial.layers"                  | "spatial.layers() :: (name :: STRING?, signature :: STRING?)"                                                                     |
| "spatial.getFeatureAttributes"    | "spatial.getFeatureAttributes(name :: STRING?) :: (name :: STRING?)"                                                              |
| "spatial.setFeatureAttributes"    | "spatial.setFeatureAttributes(name :: STRING?, attributeNames :: LIST? OF STRING?) :: (node :: NODE?)"                            |
| "spatial.addWKTLayer"             | "spatial.addWKTLayer(name :: STRING?, nodePropertyName :: STRING?) :: (node :: NODE?)"                                            |
| "spatial.closest"                 | "spatial.closest(layerName :: STRING?, coordinate :: ANY?, distanceInKm :: FLOAT?) :: (node :: NODE?)"                            |
| "spatial.importOSMToLayer"        | "spatial.importOSMToLayer(layerName :: STRING?, uri :: STRING?) :: (count :: INTEGER?)"                                           |
| "spatial.addPointLayerWithConfig" | "spatial.addPointLayerWithConfig(name :: STRING?, encoderConfig :: STRING?) :: (node :: NODE?)"                                   |
| "spatial.addPointLayer"           | "spatial.addPointLayer(name :: STRING?) :: (node :: NODE?)"                                                                       |
| "spatial.addWKT"                  | "spatial.addWKT(layerName :: STRING?, geometry :: STRING?) :: (node :: NODE?)"                                                    |
| "spatial.bbox"                    | "spatial.bbox(layerName :: STRING?, min :: ANY?, max :: ANY?) :: (node :: NODE?)"                                                 |
| "spatial.addLayer"                | "spatial.addLayer(name :: STRING?, type :: STRING?, encoderConfig :: STRING?) :: (node :: NODE?)"                                 |
| "spatial.addLayerWithEncoder"     | "spatial.addLayerWithEncoder(name :: STRING?, encoder :: STRING?, encoderConfig :: STRING?) :: (node :: NODE?)"                   |
| "spatial.addWKTs"                 | "spatial.addWKTs(layerName :: STRING?, geometry :: LIST? OF STRING?) :: (node :: NODE?)"                                          |
| "spatial.layerTypes"              | "spatial.layerTypes() :: (name :: STRING?, signature :: STRING?)"                                                                 |
| "spatial.importOSM"               | "spatial.importOSM(uri :: STRING?) :: (count :: INTEGER?)"                                                                        |
| "spatial.asGeometry"              | "spatial.asGeometry(geometry :: ANY?) :: (geometry :: ANY?)"                                                                      |
| "spatial.procedures"              | "spatial.procedures() :: (name :: STRING?, signature :: STRING?)"                                                                 |
| "spatial.withinDistance"          | "spatial.withinDistance(layerName :: STRING?, coordinate :: ANY?, distanceInKm :: FLOAT?) :: (node :: NODE?, distance :: FLOAT?)" |
+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------+

30 rows available after 41 ms, consumed after another 24 ms
LeBlue commented 7 years ago

I did not look into it further, I only needed simple features and used spatial procedures of APOC procedures.

craigtaverner commented 7 years ago

I had a quick look at noticed that the DefaultLayer class by default deletes geometry nodes when you call layer.delete - see https://github.com/neo4j-contrib/spatial/blob/master/src/main/java/org/neo4j/gis/spatial/DefaultLayer.java#L328

This seems to be as a result of the original use cases being layers as the primary model for geometry data, so deleting the layer implied deleting the data. Later work on dynamic layers and the OSM layer demonstrated a need for layers to be views onto other models, and deleting was disabled.

However, it would now be interesting to know what is the best way forward. Here are some choices:

My first impression is to go with the middle option. What do others think?

jexp commented 7 years ago

I'd suggest adding the boolean flag with a sensible default value.

Von meinem iPhone gesendet

Am 12.06.2017 um 16:20 schrieb Craig Taverner notifications@github.com:

I had a quick look at noticed that the DefaultLayer class by default deletes geometry nodes when you call layer.delete - see https://github.com/neo4j-contrib/spatial/blob/master/src/main/java/org/neo4j/gis/spatial/DefaultLayer.java#L328

This seems to be as a result of the original use cases being layers as the primary model for geometry data, so deleting the layer implied deleting the data. Later work on dynamic layers and the OSM layer demonstrated a need for layers to be views onto other models, and deleting was disabled.

However, it would now be interesting to know what is the best way forward. Here are some choices:

Change delete to not delete, but instead remove the layer (and leave the geometry nodes). This would not be backwards compatable, but is as easy as changing the true to a false on the line linked above (and fixing a lot of tests that will then fail). Expose the boolean deleteGeomNodes so that the caller needs to decide what they want (and expose this up to the procedure too) Make two methods, one for deleting the layer and geometry nodes, and one for deleting the layer without geometry nodes. My first impression is to go with the middle option. What do others think?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

LeBlue commented 7 years ago

I was just surprised by the default behaviour, so at least it should be documented which layer types "own" the added nodes and a comment to the spatial.removeLayer description, that removing the layer may delete the geometry nodes.

craigtaverner commented 7 years ago

Another idea I had was that geometry nodes created by the layer to represent geometries added to the layer should be deleted, but nodes added to the layer should not. This means if you have made your own nodes, you should feel safe adding them, but if the layer created them for you, you should not. One problem with this idea is that it is a grey area whether it is your code creating the nodes or spatial, because one of the points of spatial was to be extended by users.

nw31304 commented 7 years ago

Just to complicate matters further, is there not a “bbox” attribute added to user nodes once they are added to an index? Ideally, those attributes would be removed along with the index to save space in the store. One issue that comes to mind though…

What happens if a node has two WKT attributes, say “wkt1” and “wkt2”, where wkt1 != wkt2. Suppose the user wishes to add the node to two spatial WKT indices, one indexed on on wkt1 and another on wkt2. You obviously wouldn’t want to clean up that attribute when removing an index in that case. Now that I think about it, if there is only a single “bbox” attribute, is it even possible for a node to belong to two different indices? Wouldn’t the indexing for the later layer overwrite the “bbox" attr of the previous?

On 14/06/2017, at 2:16 AM, Craig Taverner notifications@github.com wrote:

Another idea I had was that geometry nodes created by the layer to represent geometries added to the layer should be deleted, but nodes added to the layer should not. This means if you have made your own nodes, you should feel safe adding them, but if the layer created them for you, you should not. One problem with this idea is that it is a grey area whether it is your code creating the nodes or spatial, because one of the points of spatial was to be extended by users.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/neo4j-contrib/spatial/issues/283#issuecomment-308130497, or mute the thread https://github.com/notifications/unsubscribe-auth/AEHXmkKXS1EcbI76_v3dYMMGjUfgLD8Hks5sDpm9gaJpZM4K5ik-.

craigtaverner commented 7 years ago

I remember a few years ago adding support for two wkt (and two bbox) attributes on the same node. You need to add it to two different layers, and the layers need to be configured to use different property names for both the geometry and the bbox attributes. But your point is valid, removing the node from the layer should also remove those attributes.

fs123 commented 6 years ago

We have this issue too, after we changed our model. As a workaround we just delete the relations before deleting the index by calling:

MATCH (n)-[:RTREE_REFERENCE*]->() DETACH DELETE n

Ps: actually we want to rebuild the index. We do this by deleting and (re-)creating the index.