Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mark deleted entities as deleted in the string serialization #6354

Merged
merged 11 commits into from Feb 12, 2016
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