Dynamic associations are not loaded correctly when they have instance level labels #43
Comments
|
This is the challenge with dynamic labels and dynamic associations (both features that should not exist IMO as the former prevents batch updates/optimized writes and the latter has a negative impact on read performance ). Since we cannot know for sure what domain class type the labels are associated with we cannot establish the inverse type of the association (in this case |
|
But this was working obj previous versions, how were you doing that?
El 20 de abr. de 2017 06:44 -0300, Graeme Rocher <notifications@github.com>, escribió:
…
This is the challenge with dynamic labels and dynamic associations (both features that should not exist IMO as the former prevents batch updates/optimized writes and the latter has a negative impact on read performance ).
Since we cannot know for sure what domain class type the labels are associated with we cannot establish the inverse type of the association (in this case Club). I see no way to fix this as it stands.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub (#43 (comment)), or mute the thread (https://github.com/notifications/unsubscribe-auth/AA5opon7ZM0yeQVANzX0Y7kPy9pYD2xkks5rxyj2gaJpZM4M8DTg).
|
|
I think you were using just the static labels to load the dynamic associations.
El 20 de abr. de 2017 06:44 -0300, Graeme Rocher <notifications@github.com>, escribió:
…
This is the challenge with dynamic labels and dynamic associations (both features that should not exist IMO as the former prevents batch updates/optimized writes and the latter has a negative impact on read performance ).
Since we cannot know for sure what domain class type the labels are associated with we cannot establish the inverse type of the association (in this case Club). I see no way to fix this as it stands.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub (#43 (comment)), or mute the thread (https://github.com/notifications/unsubscribe-auth/AA5opon7ZM0yeQVANzX0Y7kPy9pYD2xkks5rxyj2gaJpZM4M8DTg).
|
|
Not possible, how would you know the static labels? Given that you have no idea what is the inverse entity of the association |
|
I think you were using the targetType from the relationship?
I'm sure we have this working in 6.0.9 and grails 2.5.6 but not in 6.1 and grails 3.2.8.
El 20 de abr. de 2017 08:32 -0300, Graeme Rocher <notifications@github.com>, escribió:
…
Not possible, how would you know the static labels? Given that you have no idea what is the inverse entity of the association
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub (#43 (comment)), or mute the thread (https://github.com/notifications/unsubscribe-auth/AA5opvfgjF2BbCQ9S1rUJqV9erDl1OoTks5rx0JdgaJpZM4M8DTg).
|
|
As far as I can see So that points to a likely a data corruption problem |
|
I ran the application at https://github.com/mburak/sampleapp and no exception was thrown |
|
Hm you are in master? I just ran it again and it threw it. I think the issue is that you are using the labels instead of the target type here: But in the entitiesByLabel you are storing only the static label as key:
|
|
Application runs fine for me on master |
|
Please update the code and try again. |
|
I noticed it printed As far as I can see from the output, I can see issue #46 which is fixed already but not much else. If I restore the dynamic labels then it prints |
|
This is with the latest code? |
|
Was finally able to reproduce after making several modifications to the code. So this doesn't seem to be a new bug, the same behaviour is present in 6.0.x which is the first version to have supported Neo4j 3.x and Bolt. Downgrading the sample to any version of 6.0.x reproduces the same issue. As far as I can see there is no way to solve it reliably since IMO It is another example of why you should stay away from both dynamic labels and dynamic associations. You can of course choose to ignore my recommendation, but if you want your applications to perform well and work consistently I recommend you don't use them. I believe we should in fact deprecate support for them and remove them completely in the future as they are not something we believe should be used by any user going forward. |
|
Ok, It's weird that we are not getting that with older version as we've been using that for a long time and it's working. |
|
And also, the problem with this is that it's happening with static associations too, so it doesn't matter if it's static or dynamic. If you have a dynamic label, it will throw the exception: Club has dynamic(instance) labels |
|
I think that one of the issues is that the query built here: |
|
That is also the same as in 6.0.x and probably earlier versions. The query format hasn't changed much for dynamic associations |
|
Well but you didn't have the DynamicAssociationSupport and the propertyMissing going through that class and doing those queries. |
|
The code just moved from Neo4jEntityPersister to DynamicAssociationSupport but the query format is the same. Like I said changing the version in the the same app to any version of 6.0.x produces the same issue, so since this is not a regression and I see no way to fix this reliably I don't see anyway to take it forward. |
|
The only change we can possibly make is changing this line: if(associatedEntity == null) {
throw new NonPersistentTypeException(labels.toString());
}To if(associatedEntity == null) {
continue
}That would at least ignore the exception for other dynamic properties. In terms of establishing which entity the associated relates to, I don't see a way to do that. |
|
Avoiding the exception would help, but also even when it was doing the same before, shouldn't the DynamicAddociationSupport just retrieve and set only dynamic associations instead of all?
El 21 de abr. de 2017 04:12 -0300, Graeme Rocher <notifications@github.com>, escribió:
…
The only change we can possibly make is changing this line:
if(associatedEntity == null) { throw new NonPersistentTypeException(labels.toString()); }
To
if(associatedEntity == null) { continue }
That would at least ignore the exception for other dynamic properties. In terms of establishing which entity the associated relates to, I don't see a way to do that.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub (#43 (comment)), or mute the thread (https://github.com/notifications/unsubscribe-auth/AA5opj1k8E3EcuCuffRG_l4ULYbnSqQaks5ryFbjgaJpZM4M8DTg).
|
|
Agreed it should, but how would you tell which ones where dynamic and which ones were not in the query? Also that behaviour is the same as 6.0.x so that is again not a change or regression in behaviour. Like I said the query hasn't changed. Just moved. It was here before https://github.com/grails/gorm-neo4j/blob/6.0.x/grails-datastore-gorm-neo4j/src/main/groovy/org/grails/datastore/gorm/neo4j/engine/Neo4jEntityPersister.java#L53 |
|
I think that dynamic associations had sourceType and targetType and static ones didn't before.
El 21 de abr. de 2017 06:19 -0300, Graeme Rocher <notifications@github.com>, escribió:
…
Agreed it should, but how would you tell which ones where dynamic and which ones were not in the query? Also that behaviour is the same as 6.0.x so that is again not a change or regression in behaviour.
Like I said the query hasn't changed. Just moved. It was here before https://github.com/grails/gorm-neo4j/blob/6.0.x/grails-datastore-gorm-neo4j/src/main/groovy/org/grails/datastore/gorm/neo4j/engine/Neo4jEntityPersister.java#L53
Now it is here https://github.com/grails/gorm-neo4j/blob/master/grails-datastore-gorm-neo4j/src/main/groovy/org/grails/datastore/gorm/neo4j/GraphPersistentEntity.groovy#L216
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub (#43 (comment)), or mute the thread (https://github.com/notifications/unsubscribe-auth/AA5opjTta4f0aeXNsB0cOIg9Evq1GVZ-ks5ryHSYgaJpZM4M8DTg).
|
|
static associations still don't have sourceType and targetType. They are only included for dynamic associations https://github.com/grails/gorm-neo4j/blob/master/grails-datastore-gorm-neo4j/src/main/groovy/org/grails/datastore/gorm/neo4j/engine/RelationshipPendingInsert.java#L110 Of course if you have data that you have previously inserted via dynamic associations there is nothing we can do about that. You will have to migrate the data manually |
|
I suggest since you have the data model in place you work on a solution that solves the issue for you and a submit a pull request. Thanks. |
|
Sounds good.
… El 21 abr. 2017, a las 12:32, Graeme Rocher ***@***.***> escribió:
I suggest since you have the data model in place you work on a solution that solves the issue for you and a submit a pull request. Thanks.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub <#43 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA5opgDJi3iYBQrNgQN5mSa7AbqxZ7dLks5ryMwNgaJpZM4M8DTg>.
|
We have the following classes:
We set a dynamic property called "club" to a player object. Then when we try to access that property we are getting the following exception:
You can reproduce that by running the project in https://github.com/mburak/sampleapp
The text was updated successfully, but these errors were encountered: