From 55d7ac45a0c0d07d23acedf9f94b6c789316850e Mon Sep 17 00:00:00 2001 From: fickludd Date: Thu, 6 Sep 2018 08:28:48 +0200 Subject: [PATCH] review comment cleanups --- .../v3_5/logical/plans/CachedNodeProperty.scala | 17 +++++++++++------ .../TransactionBoundQueryContext.scala | 2 +- .../org/neo4j/internal/kernel/api/Read.java | 2 +- .../kernel/api/TransactionStateTestBase.java | 10 +++++----- .../internal/kernel/api/helpers/StubRead.java | 2 +- .../api/state/PropertyContainerStateImpl.java | 6 +----- .../kernel/impl/newapi/AllStoreHolder.java | 2 +- 7 files changed, 21 insertions(+), 20 deletions(-) diff --git a/community/cypher/cypher-logical-plans-3.5/src/main/scala/org/neo4j/cypher/internal/v3_5/logical/plans/CachedNodeProperty.scala b/community/cypher/cypher-logical-plans-3.5/src/main/scala/org/neo4j/cypher/internal/v3_5/logical/plans/CachedNodeProperty.scala index 63aa15d14f47..5717f1a9244c 100644 --- a/community/cypher/cypher-logical-plans-3.5/src/main/scala/org/neo4j/cypher/internal/v3_5/logical/plans/CachedNodeProperty.scala +++ b/community/cypher/cypher-logical-plans-3.5/src/main/scala/org/neo4j/cypher/internal/v3_5/logical/plans/CachedNodeProperty.scala @@ -19,8 +19,9 @@ */ package org.neo4j.cypher.internal.v3_5.logical.plans -import org.opencypher.v9_0.expressions.{LogicalVariable, PropertyKeyName} -import org.opencypher.v9_0.util.InputPosition +import org.opencypher.v9_0.ast.semantics.{SemanticCheck, SemanticCheckResult, SemanticCheckableExpression} +import org.opencypher.v9_0.expressions.{LogicalVariable, PropertyKeyName, Expression => ASTExpression} +import org.opencypher.v9_0.util.{InputPosition, InternalException} /** * A node property value that is cached in the execution context. Such a value can be @@ -33,7 +34,7 @@ import org.opencypher.v9_0.util.InputPosition */ case class CachedNodeProperty(nodeVariableName: String, propertyKey: PropertyKeyName - )(val position: InputPosition) extends LogicalVariable { + )(val position: InputPosition) extends LogicalVariable with SemanticCheckableExpression { def asString = s"$nodeVariableName.${propertyKey.name}" @@ -41,9 +42,13 @@ case class CachedNodeProperty(nodeVariableName: String, override def asCanonicalStringVal: String = s"`$name`" - override def copyId: LogicalVariable = ??? + override def semanticCheck(ctx: ASTExpression.SemanticContext): SemanticCheck = SemanticCheckResult.success - override def renameId(newName: String): LogicalVariable = ??? + override def copyId = fail() - override def bumpId: LogicalVariable = ??? + override def renameId(newName: String) = fail() + + override def bumpId = fail() + + private def fail(): Nothing = throw new InternalException("Tried using a CachedNodeProperty as Variable") } diff --git a/community/cypher/interpreted-runtime/src/main/scala/org/neo4j/cypher/internal/runtime/interpreted/TransactionBoundQueryContext.scala b/community/cypher/interpreted-runtime/src/main/scala/org/neo4j/cypher/internal/runtime/interpreted/TransactionBoundQueryContext.scala index c8cf54aafd9b..ff24d41fe8f1 100644 --- a/community/cypher/interpreted-runtime/src/main/scala/org/neo4j/cypher/internal/runtime/interpreted/TransactionBoundQueryContext.scala +++ b/community/cypher/interpreted-runtime/src/main/scala/org/neo4j/cypher/internal/runtime/interpreted/TransactionBoundQueryContext.scala @@ -579,7 +579,7 @@ sealed class TransactionBoundQueryContext(val transactionalContext: Transactiona } override def getTxStateProperty(nodeId: Long, propertyKeyId: Int): Option[Value] = { - val nodePropertyInTx = reads().nodePropertyChangeInTransaction(nodeId, propertyKeyId) + val nodePropertyInTx = reads().nodePropertyChangeInTransactionOrNull(nodeId, propertyKeyId) Option(nodePropertyInTx) } diff --git a/community/kernel-api/src/main/java/org/neo4j/internal/kernel/api/Read.java b/community/kernel-api/src/main/java/org/neo4j/internal/kernel/api/Read.java index 23552b979b94..9c023b964451 100644 --- a/community/kernel-api/src/main/java/org/neo4j/internal/kernel/api/Read.java +++ b/community/kernel-api/src/main/java/org/neo4j/internal/kernel/api/Read.java @@ -331,7 +331,7 @@ long lockingNodeUniqueIndexSeek( IndexReference index, IndexQuery.ExactPredicate * @return null if the property has not been changed for the node in this transaction. Otherwise returns * the new property value, or {@link Values#NO_VALUE} if the property has been removed in this transaction. */ - Value nodePropertyChangeInTransaction( long node, int propertyKeyId ); + Value nodePropertyChangeInTransactionOrNull( long node, int propertyKeyId ); void graphProperties( PropertyCursor cursor ); diff --git a/community/kernel-api/src/test/java/org/neo4j/internal/kernel/api/TransactionStateTestBase.java b/community/kernel-api/src/test/java/org/neo4j/internal/kernel/api/TransactionStateTestBase.java index 086903325b29..203e9205053e 100644 --- a/community/kernel-api/src/test/java/org/neo4j/internal/kernel/api/TransactionStateTestBase.java +++ b/community/kernel-api/src/test/java/org/neo4j/internal/kernel/api/TransactionStateTestBase.java @@ -119,19 +119,19 @@ public void shouldReportInTransactionNodeProperty() throws Exception // THEN assertNull( "Unchanged existing property is null", - tx.dataRead().nodePropertyChangeInTransaction( node, p1 ) ); + tx.dataRead().nodePropertyChangeInTransactionOrNull( node, p1 ) ); assertNull( "Unchanged missing property is null", - tx.dataRead().nodePropertyChangeInTransaction( node, p2 ) ); + tx.dataRead().nodePropertyChangeInTransactionOrNull( node, p2 ) ); assertEquals( "Changed property is new value", Values.of( 13 ), - tx.dataRead().nodePropertyChangeInTransaction( node, p3 ) ); + tx.dataRead().nodePropertyChangeInTransactionOrNull( node, p3 ) ); assertEquals( "Removed property is NO_VALUE", Values.NO_VALUE, - tx.dataRead().nodePropertyChangeInTransaction( node, p4 ) ); + tx.dataRead().nodePropertyChangeInTransactionOrNull( node, p4 ) ); assertEquals( "Added property is new value", Values.of( 15 ), - tx.dataRead().nodePropertyChangeInTransaction( node, p5 ) ); + tx.dataRead().nodePropertyChangeInTransactionOrNull( node, p5 ) ); } } } diff --git a/community/kernel-api/src/test/java/org/neo4j/internal/kernel/api/helpers/StubRead.java b/community/kernel-api/src/test/java/org/neo4j/internal/kernel/api/helpers/StubRead.java index ede27c7ea876..74dd16a3d4cf 100644 --- a/community/kernel-api/src/test/java/org/neo4j/internal/kernel/api/helpers/StubRead.java +++ b/community/kernel-api/src/test/java/org/neo4j/internal/kernel/api/helpers/StubRead.java @@ -212,7 +212,7 @@ public boolean relationshipDeletedInTransaction( long relationship ) } @Override - public Value nodePropertyChangeInTransaction( long node, int propertyKeyId ) + public Value nodePropertyChangeInTransactionOrNull( long node, int propertyKeyId ) { throw new UnsupportedOperationException(); } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/state/PropertyContainerStateImpl.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/state/PropertyContainerStateImpl.java index ce07ea5a0889..f8cf81c0ab8c 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/api/state/PropertyContainerStateImpl.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/api/state/PropertyContainerStateImpl.java @@ -190,11 +190,7 @@ public Value propertyValue( int propertyKey ) } if ( changedProperties != null ) { - Value changedValue = changedProperties.get( propertyKey ); - if ( changedValue != null ) - { - return changedValue; - } + return changedProperties.get( propertyKey ); } return null; } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/AllStoreHolder.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/AllStoreHolder.java index d410f2dadafe..f55ba2a877ed 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/AllStoreHolder.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/newapi/AllStoreHolder.java @@ -146,7 +146,7 @@ public boolean relationshipDeletedInTransaction( long relationship ) } @Override - public Value nodePropertyChangeInTransaction( long node, int propertyKeyId ) + public Value nodePropertyChangeInTransactionOrNull( long node, int propertyKeyId ) { ktx.assertOpen(); return hasTxStateWithChanges() ? txState().getNodeState( node ).propertyValue( propertyKeyId ) : null;