Skip to content

Commit

Permalink
Merge pull request #6354 from Mats-SX/3.0-deleted-entities-serializat…
Browse files Browse the repository at this point in the history
…ion2

Mark deleted entities as deleted in the string serialization
  • Loading branch information
systay committed Feb 12, 2016
2 parents 3cad6b4 + ac23b4d commit 1c6e22f
Show file tree
Hide file tree
Showing 34 changed files with 1,858 additions and 275 deletions.
Expand Up @@ -21,13 +21,45 @@ package org.neo4j.internal.cypher.acceptance

import org.neo4j.cypher.internal.compiler.v3_0.commands.expressions.PathImpl
import org.neo4j.cypher.internal.compiler.v3_0.test_helpers.CustomMatchers
import org.neo4j.cypher.{ExecutionEngineFunSuite, NewPlannerTestSupport, SyntaxException}
import org.neo4j.cypher.{EntityNotFoundException, ExecutionEngineFunSuite, NewPlannerTestSupport, SyntaxException}
import org.neo4j.graphdb._

import scala.util.Random

class ReturnAcceptanceTest extends ExecutionEngineFunSuite with CustomMatchers with NewPlannerTestSupport {

test("returning properties of deleted nodes should throw exception") {
createNode("p" -> 0)

val query = "MATCH (n) DELETE n RETURN n.p"

an [EntityNotFoundException] should be thrownBy updateWithBothPlanners(query)
}

test("returning labels of deleted nodes should throw exception") {
createLabeledNode("A")

val query = "MATCH (n:A) DELETE n RETURN labels(n)"

an [EntityNotFoundException] should be thrownBy updateWithBothPlanners(query)
}

test("returning properties of deleted relationships should throw exception") {
relate(createNode(), createNode(), "T", Map("p" -> "a property"))

val query = "MATCH ()-[r]->() DELETE r RETURN r.p"

an [EntityNotFoundException] should be thrownBy updateWithBothPlanners(query)
}

test("returning the type of deleted relationships should throw exception") {
relate(createNode(), createNode(), "T")

val query = "MATCH ()-[r:T]->() DELETE r RETURN type(r)"

an [EntityNotFoundException] should be thrownBy updateWithBothPlanners(query)
}

test("should choke on an invalid unicode literal") {
val query = "RETURN '\\uH' AS a"

Expand Down
@@ -0,0 +1,134 @@
/*
* Copyright (c) 2002-2016 "Neo Technology,"
* Network Engine for Objects in Lund AB [http://neotechnology.com]
*
* This file is part of Neo4j.
*
* Neo4j is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
package org.neo4j.internal.cypher.acceptance

import org.neo4j.cypher._

class SerializationAcceptanceTest extends ExecutionEngineFunSuite with QueryStatisticsTestSupport {

// serialization of deleted entities

test("deleted nodes should be returned marked as such") {
createNode()

val query = "MATCH (n) DELETE n RETURN n"

graph.inTx {
val result = execute(query)

result.dumpToString() should include("Node[0]{deleted}")
}
}

test("non-deleted nodes should be returned as normal") {
createNode()

val query = "MATCH (n) RETURN n"

graph.inTx {
val result = execute(query)

result.dumpToString() should not include "deleted"
}
}

test("non-deleted relationships should be returned as normal") {
relate(createNode(), createNode(), "T")

val query = "MATCH ()-[r]->() RETURN r"

graph.inTx {
val result = execute(query)

result.dumpToString() should not include "deleted"
}
}

test("deleted relationships should be returned marked as such") {
relate(createNode(), createNode(), "T")

val query = "MATCH ()-[r]->() DELETE r RETURN r"

graph.inTx {
val result = execute(query)

result.dumpToString() should include(":T[0]{deleted}")
}
}

test("returning everything when including deleted entities should work") {
relate(createNode(), createNode(), "T")

val query = "MATCH (a)-[r]->(b) DELETE a, r, b RETURN *"

graph.inTx {
val result = execute(query)

result.dumpToString() should include(":T[0]{deleted}")
result.dumpToString() should include("Node[0]{deleted}")
result.dumpToString() should include("Node[1]{deleted}")
}
}

test("returning a deleted path") {
relate(createNode(), createNode(), "T")

val query = "MATCH p=(a)-[r]->(b) DELETE p RETURN p"

graph.inTx {
val result = execute(query)

result.dumpToString() should include(":T[0]{deleted}")
result.dumpToString() should include("Node[0]{deleted}")
result.dumpToString() should include("Node[1]{deleted}")
}
}

test("returning a deleted path with deleted node") {
relate(createNode(), createNode(), "T")

val query = "MATCH p=(a)-[r]->(b) DELETE a, r RETURN p"

graph.inTx {
val result = execute(query)

println(result.dumpToString())

result.dumpToString() should include(":T[0]{deleted}")
result.dumpToString() should include("Node[0]{deleted}")
result.dumpToString() should not include("Node[1]{deleted}")
}
}

test("returning a deleted path with deleted relationship") {
relate(createNode(), createNode(), "T")

val query = "MATCH p=(a)-[r]->(b) DELETE r RETURN p"

graph.inTx {
val result = execute(query)

result.dumpToString() should include(":T[0]{deleted}")
result.dumpToString() should not include("Node[0]{deleted}")
result.dumpToString() should not include("Node[1]{deleted}")
}
}

}
Expand Up @@ -30,12 +30,13 @@ import scala.collection.Map
trait CypherSerializer {

protected def serializeProperties(x: PropertyContainer, qtx: QueryContext): String = {
val (ops, id) = x match {
case n: Node => (qtx.nodeOps, n.getId)
case r: Relationship => (qtx.relationshipOps, r.getId)
val (ops, id, deleted) = x match {
case n: Node => (qtx.nodeOps, n.getId, qtx.nodeOps.isDeletedInThisTx(n))
case r: Relationship => (qtx.relationshipOps, r.getId, qtx.relationshipOps.isDeletedInThisTx(r))
}

val keyValStrings = ops.propertyKeyIds(id).
val keyValStrings = if (deleted) Iterator("deleted")
else ops.propertyKeyIds(id).
map(pkId => qtx.getPropertyKeyName(pkId) + ":" + serialize(ops.getProperty(id, pkId), qtx))

keyValStrings.mkString("{", ",", "}")
Expand Down
Expand Up @@ -21,14 +21,23 @@ package org.neo4j.cypher.internal.compiler.v3_0.commands.expressions

import org.neo4j.cypher.internal.compiler.v3_0._
import org.neo4j.cypher.internal.compiler.v3_0.executionplan.{Effects, ReadsAllRelationships}
import org.neo4j.cypher.internal.compiler.v3_0.helpers.CastSupport
import org.neo4j.cypher.internal.compiler.v3_0.pipes.QueryState
import org.neo4j.cypher.internal.compiler.v3_0.symbols.SymbolTable
import org.neo4j.cypher.internal.frontend.v3_0.{CypherTypeException, EntityNotFoundException}
import org.neo4j.cypher.internal.frontend.v3_0.symbols._
import org.neo4j.graphdb.Relationship

case class RelationshipTypeFunction(relationship: Expression) extends NullInNullOutExpression(relationship) {
def compute(value: Any, m: ExecutionContext)(implicit state: QueryState) =
value.asInstanceOf[Relationship].getType.name()

def compute(value: Any, m: ExecutionContext)(implicit state: QueryState): String = {
val relationship = CastSupport.castOrFail[Relationship](value)
if (state.query.relationshipOps.isDeletedInThisTx(relationship)) {
throw new EntityNotFoundException(s"Relationship with id ${relationship.getId} has been deleted in this transaction")
} else {
relationship.getType.name()
}
}

def rewrite(f: (Expression) => Expression) = f(RelationshipTypeFunction(relationship.rewrite(f)))

Expand Down
Expand Up @@ -46,15 +46,15 @@ case class DeleteEntityAction(elementToDelete: Expression, forced: Boolean)

private def delete(x: graphdb.PropertyContainer, state: QueryState, forced: Boolean) {
x match {
case n: graphdb.Node if !state.query.nodeOps.isDeleted(n) && forced =>
case n: graphdb.Node if !state.query.nodeOps.isDeletedInThisTx(n) && forced =>
val rels = state.query.getRelationshipsForIds(n, SemanticDirection.BOTH, None)
rels.foreach(r => delete(r, state, forced))
state.query.nodeOps.delete(n)

case n: graphdb.Node if !state.query.nodeOps.isDeleted(n) =>
case n: graphdb.Node if !state.query.nodeOps.isDeletedInThisTx(n) =>
state.query.nodeOps.delete(n)

case r: graphdb.Relationship if !state.query.relationshipOps.isDeleted(r) =>
case r: graphdb.Relationship if !state.query.relationshipOps.isDeletedInThisTx(r) =>
state.query.relationshipOps.delete(r)

case _ =>
Expand Down
Expand Up @@ -51,7 +51,7 @@ case class DeletePipe(src: Pipe, expression: Expression, forced: Boolean)(val es
}
}

private def deleteNode(n: Node)(implicit state: QueryState) = if (!state.query.nodeOps.isDeleted(n)) {
private def deleteNode(n: Node)(implicit state: QueryState) = if (!state.query.nodeOps.isDeletedInThisTx(n)) {
if (forced) {
val relationships = state.query.getRelationshipsForIds(n, SemanticDirection.BOTH, None)
relationships.foreach(deleteRelationship)
Expand All @@ -60,7 +60,7 @@ case class DeletePipe(src: Pipe, expression: Expression, forced: Boolean)(val es
}

private def deleteRelationship(r: Relationship)(implicit state: QueryState) =
if (!state.query.relationshipOps.isDeleted(r)) state.query.relationshipOps.delete(r)
if (!state.query.relationshipOps.isDeletedInThisTx(r)) state.query.relationshipOps.delete(r)

private def deletePath(p: Path)(implicit state: QueryState) = p.iterator().asScala.foreach {
case n: Node =>
Expand Down
Expand Up @@ -227,7 +227,7 @@ class DelegatingOperations[T <: PropertyContainer](protected val inner: Operatio

override def all: Iterator[T] = manyDbHits(inner.all)

override def isDeleted(obj: T): Boolean = inner.isDeleted(obj)
override def isDeletedInThisTx(obj: T): Boolean = inner.isDeletedInThisTx(obj)

override def acquireExclusiveLock(obj: Long): Unit = inner.acquireExclusiveLock(obj)

Expand Down
Expand Up @@ -190,7 +190,7 @@ trait Operations[T <: PropertyContainer] {

def indexQuery(name: String, query: Any): Iterator[T]

def isDeleted(obj: T): Boolean
def isDeletedInThisTx(obj: T): Boolean

def all: Iterator[T]

Expand Down

0 comments on commit 1c6e22f

Please sign in to comment.