Copilot commented on code in PR #15833:
URL: https://github.com/apache/grails-core/pull/15833#discussion_r3525918388
##########
grails-data-neo4j/grails-datastore-gorm-neo4j/src/main/groovy/org/grails/datastore/gorm/neo4j/engine/Neo4jAssociationQueryExecutor.groovy:
##########
@@ -83,50 +81,47 @@ class Neo4jAssociationQueryExecutor implements
AssociationQueryExecutor<Serializ
@Override
List<Object> query(Serializable primaryKey) {
- QueryRunner statementRunner = session.hasTransaction() ?
session.getTransaction().getNativeTransaction() :
(Session)session.nativeInterface
+ QueryRunner statementRunner = session.hasTransaction() ?
session.getTransaction().getNativeTransaction() : (Session)
session.nativeInterface
String relType
- GraphPersistentEntity parent = (GraphPersistentEntity)association.owner
- GraphPersistentEntity related = (GraphPersistentEntity)indexedEntity
+ GraphPersistentEntity parent = (GraphPersistentEntity)
association.owner
+ GraphPersistentEntity related = (GraphPersistentEntity) indexedEntity
boolean isRelationship = related.isRelationshipEntity()
- if(isRelationship) {
- RelationshipPersistentEntity relEntity =
(RelationshipPersistentEntity)related
+ if (isRelationship) {
+ RelationshipPersistentEntity relEntity =
(RelationshipPersistentEntity) related
GraphPersistentEntity fromEntity = (GraphPersistentEntity)
relEntity.getFrom().getAssociatedEntity()
GraphPersistentEntity toEntity = (GraphPersistentEntity)
relEntity.getTo().getAssociatedEntity()
- if(parent == fromEntity) {
+ if (parent == fromEntity) {
relType = "-[rel]->"
related = toEntity
- }
- else {
+ } else {
relType = "<-[rel]-"
parent = toEntity
related = fromEntity
}
- }
- else {
+ } else {
relType = RelationshipUtils.matchForAssociation(association)
}
String relationship =
CypherBuilder.buildRelationship(parent.labelsAsString, relType,
related.labelsAsString)
StringBuilder cypher = new
StringBuilder(CypherBuilder.buildRelationshipMatch(parent.labelsAsString,
relType, related.labelsAsString))
cypher.append('( ')
- .append(parent.formatId(RelationshipPersistentEntity.FROM))
- .append(" = \$id )")
+ .append(parent.formatId(RelationshipPersistentEntity.FROM))
+ .append(" = \$id )")
boolean isLazyToMany = lazy && !isRelationship && association
instanceof ToMany
- if(isLazyToMany) {
+ if (isLazyToMany) {
cypher.append(related.formatId(RelationshipPersistentEntity.TO))
- .append("RETURN as id")
- }
- else {
- if(!isRelationship) {
+ .append("RETURN as id")
Review Comment:
The lazy-to-many Cypher query being built here is malformed: it produces
`...ID(to)RETURN as id` (missing whitespace and missing the return expression).
This will fail at runtime when lazy-loading a to-many association. You can use
GraphPersistentEntity#formatReturnId to generate a correct `RETURN <id> as id`
clause.
##########
grails-data-neo4j/grails-datastore-gorm-neo4j/src/main/groovy/org/grails/datastore/gorm/neo4j/engine/Neo4jAssociationQueryExecutor.groovy:
##########
@@ -137,32 +132,31 @@ class Neo4jAssociationQueryExecutor implements
AssociationQueryExecutor<Serializ
}
associations.addAll(entity.associations)
- if(associations.size() > 0) {
+ if (associations.size() > 0) {
int i = 0
List previousAssociations = []
- for(Association association in associations) {
- if(association.isBasic()) continue
+ for (Association association in associations) {
+ if (association.isBasic()) continue
boolean isEager = ((Property)
association.mapping.mappedForm).isLazy()
Review Comment:
`Property.isLazy()` only indicates whether lazy proxies are used for
collection elements; it is not an "eager" indicator. Using it for `isEager`
inverts the logic below and can cause eager associations to be handled as lazy
(and vice versa). `isEager` should be derived from the mapping fetch strategy
instead.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
