Skip to content

Commit

Permalink
Merge remote-tracking branch 'upstream/3.3' into 3.4
Browse files Browse the repository at this point in the history
  • Loading branch information
pontusmelke committed Jan 15, 2018
2 parents bcfd4e5 + 21136e6 commit 400b344
Show file tree
Hide file tree
Showing 5 changed files with 93 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1062,7 +1062,7 @@ public Value nodeSetProperty( KernelStatement state, long nodeId, int propertyKe
}
else
{
if ( !value.equals( existingValue ) )
if ( propertyHasChanged( value, existingValue ) )
{
state.txState().nodeDoChangeProperty( node.id(), propertyKeyId, existingValue, value );
indexTxStateUpdater.onPropertyChange( state, node, propertyKeyId, existingValue, value );
Expand Down Expand Up @@ -1094,7 +1094,7 @@ public Value relationshipSetProperty( KernelStatement state, long relationshipId
ops, relationshipId, propertyKeyId, existingValue, value );
}
}
if ( !value.equals( existingValue ) )
if ( propertyHasChanged( value, existingValue ) )
{
state.txState().relationshipDoReplaceProperty(
relationship.id(), propertyKeyId, existingValue, value );
Expand Down Expand Up @@ -1812,4 +1812,12 @@ private int computeDegree( KernelStatement statement, NodeItem node, Direction
: storeLayer.nodeGetRelationships( storeStatement, node, direction, t -> t == relType ) );
}
}

private boolean propertyHasChanged( Value lhs, Value rhs )
{
//It is not enough to check equality here since by our equality semantics `int == tofloat(int)` is `true`
//so by only checking for equality users cannot change type of property without also "changing" the value.
//Hence the extra type check here.
return lhs.getClass() != rhs.getClass() || !lhs.equals( rhs );
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ public Value nodeSetProperty( long node, int propertyKey, Value value )
}
else
{
if ( !value.equals( existingValue ) )
if ( propertyHasChanged( value, existingValue ) )
{
//the value has changed to a new value
autoIndexing.nodes().propertyChanged( this, node, propertyKey, existingValue, value );
Expand Down Expand Up @@ -563,6 +563,14 @@ private void acquireSharedLabelLock( int labelId )
ktx.locks().optimistic().acquireShared( ktx.lockTracer(), ResourceTypes.LABEL, labelId );
}

private boolean propertyHasChanged( Value lhs, Value rhs )
{
//It is not enough to check equality here since by our equality semantics `int == tofloat(int)` is `true`
//so by only checking for equality users cannot change type of property without also "changing" the value.
//Hence the extra type check here.
return lhs.getClass() != rhs.getClass() || !lhs.equals( rhs );
}

public ExplicitIndexRead indexRead()
{
return allStoreHolder;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,32 @@ public void getAllPropertiesShouldWorkFineWithConcurrentPropertyModifications()
}
}

@Test
public void shouldBeAbleToForceTypeChangeOfProperty()
{
// Given
Node node;
try ( Transaction tx = db.beginTx() )
{
node = db.createNode();
node.setProperty( "prop", 1337 );
tx.success();
}

// When
try ( Transaction tx = db.beginTx() )
{
node.setProperty( "prop", 1337.0 );
tx.success();
}

// Then
try ( Transaction ignore = db.beginTx() )
{
assertThat( node.getProperty( "prop" ), instanceOf( Double.class ) );
}
}

private void createNodeWith( String key )
{
try ( Transaction tx = db.beginTx() )
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
import org.neo4j.graphdb.Transaction;
import org.neo4j.kernel.impl.core.RelationshipProxy.RelationshipActions;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.instanceOf;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
Expand Down Expand Up @@ -196,6 +198,32 @@ public void createDropRelationshipLongArrayProperty()
}
}

@Test
public void shouldBeAbleToForceTypeChangeOfProperty()
{
// Given
Relationship relationship;
try ( Transaction tx = db.beginTx() )
{
relationship = db.createNode().createRelationshipTo( db.createNode(), withName( "R" ) );
relationship.setProperty( "prop", 1337 );
tx.success();
}

// When
try ( Transaction tx = db.beginTx() )
{
relationship.setProperty( "prop", 1337.0 );
tx.success();
}

// Then
try ( Transaction ignore = db.beginTx() )
{
assertThat( relationship.getProperty( "prop" ), instanceOf( Double.class ) );
}
}

private void verifyIds( RelationshipActions actions, long relationshipId, long nodeId1, int typeId, long nodeId2 )
{
RelationshipProxy proxy = new RelationshipProxy( actions, relationshipId, nodeId1, typeId, nodeId2 );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,26 @@ class SetAcceptanceTest extends ExecutionEngineFunSuite with QueryStatisticsTest
result.toList should be(List(Map("n2" -> n2)))
}

test("should be able to force a type change of a node property") {
// given
createNode("prop" -> 1337)

// when
executeWith(expectedToSucceed, "MATCH (n) SET n.prop = tofloat(n.prop)")

executeWith(Configs.All, "MATCH (n) RETURN n.prop").next()("n.prop") shouldBe a[java.lang.Double]
}

test("should be able to force a type change of a relationship property") {
// given
relate(createNode(), createNode(), "prop" -> 1337)

// when
executeWith(expectedToSucceed, "MATCH ()-[r]->() SET r.prop = tofloat(r.prop)")

executeWith(Configs.All, "MATCH ()-[r]->() RETURN r.prop").next()("r.prop") shouldBe a[java.lang.Double]
}

test("should be able to set property to collection") {
// given
val node = createNode()
Expand Down

0 comments on commit 400b344

Please sign in to comment.