review comment cleanups

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,17 +34,21 @@ 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}"
 
   override def name: String = s"$nodeVariableName.${propertyKey.name}"
 
   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")
 }

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)
     }
 

community/kernel-api/src/main/java/org/neo4j/internal/kernel/api/Read.java

@@ -331,7 +331,7 @@ long lockingNodeUniqueIndexSeek( IndexReference index, IndexQuery.ExactPredicate
      * @return <code>null</code> 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 );
 

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 ) );
         }
     }
 }

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();
     }

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;
     }

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;