Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mapping of nodes and relations returned as lists or maps #737

Closed
steffen-harbich-cognitum opened this issue Jan 10, 2020 · 12 comments
Closed
Assignees

Comments

@steffen-harbich-cognitum

Expected Behavior

Consider the following query:

MATCH (n:A) RETURN n, [(n)-[rn:REL_1]-(n2) | [rn, n2]]

I would expect OGM to map n2 to its @NodeEntity object and wire relations accordingly. Alternatives:

MATCH (n:A) RETURN n, [[(n)-[rn:REL_1]-(n2) | [rn, n2]]]
MATCH (n:A) RETURN n, [(n)-[rn:REL_1]-(n2) | {rn:rn, n2:n2}]

Current Behavior

None of the above queries is mapping the node n2 when calling session.query("...", Map.of(), true).

Possible Solution

If adapted to

MATCH (n:A) RETURN n, [(n)-[:REL_1]-(n2) | n2], [(n)-[rn:REL_1]-() | rn]

then it works but IMO this is an unnecessary repitition of the pattern comprehension. Please also note that the cypher generated by OGM when calling session.load has the same pattern as the first alternative query from above which implied my assumption that it should work.

Steps to Reproduce (for bugs)

Simple example project reproducing the issue:
demo.zip

Start gradle task :test in debug mode to fail the assertion, important code lines in NodeService class.

Context

See https://stackoverflow.com/questions/59481997 for background information.

Your Environment

  • OGM Version used: 3.2.3
  • Java Version used: 11
  • Neo4J Version used: 3.5
@michael-simons
Copy link
Collaborator

I wonder if that is related to #666. Could you please test if your issues was there in 3.2.3 respectively 3.1.15?

@steffen-harbich-cognitum
Copy link
Author

I don't think this is related to #666 because here (#737) it's a even a step before the actual wiring of @NodeEntity. Consider the case that REL_1 is not even modeled in my @NodeEntity but exists in the graph. Now if I query like above I still want to get the related node n2 to be mapped by OGM but of course OGM couldn't wire returned @NodeEntity with node n. And in fact that works when return statement is a list of nodes in contrast to list of lists.

In a nutshell, this issue is about OGM mapping "list of nodes" correctly but not "list of list of nodes" or "list of maps containing nodes" in query results.

Btw my demo project for reproduction uses OGM 3.2.3.

@michael-simons michael-simons self-assigned this Jan 13, 2020
@michael-simons
Copy link
Collaborator

I cannot reproduce that issue in current 3.2 and master. I have attached an updated version of your demo project, that uses Neo4j-OGM 3.2.4 and it runs as well (
fixed-demo.zip)

See my article here if you need help upgrading Spring managed versions in you projects: https://michael-simons.github.io/simple-meetup/overriding-spring-managed-versions-in-gradle-projects.

Thanks for reporting.

@michael-simons
Copy link
Collaborator

For the record: Also fixed in Neo4j-OGM 3.1.16 (see
fixed-demo_boot2.1.11_ogm3.1.16.zip)

@steffen-harbich-cognitum
Copy link
Author

Thanks! In fact, it is working with OGM version 3.2.4, so I will update. And thanks for the spring boot dependency tip, I didn't know it was that easy to adapt the version.

@michael-simons
Copy link
Collaborator

michael-simons commented Jan 13, 2020

If you can wait till tomorrow, we will have 3.2.6 out. It fixes a performance and spring boot dev tools issue.

The upgrade with Maven is similar (use Maven properties in the pom.xml like <neo4j-ogm.version>3.2.6</neo4j-ogm.version>).

Thanks for your feedback, much appreciated.

@steffen-harbich-cognitum
Copy link
Author

@michael-simons, is that intented that the query

MATCH (n:A) RETURN n, [(n)-[rn:REL_1]-(n2) | [rn, n2]]

will return only the relation rn as object but not n2 in list [rn, n2]? In my case I really need that object n2 because the relation rn is not modeled in my @NodeEntity (see again my SO question https://stackoverflow.com/questions/59481997, I need arbitrary relations between my @NodeEntities).

Tested with 3.2.6.

meistermeier added a commit that referenced this issue Jan 15, 2020
If we encounter a list result that contains relationships
and nodes from a pattern comprehension both types of object will get
bundled into the OGM result.
@michael-simons
Copy link
Collaborator

Thanks to the awesome @meistermeier I understood your problem better and we fixed that, too (and yes, we do think that this is a bug).

@steffen-harbich-cognitum
Copy link
Author

Great news, so this will be included in 3.2.7?

meistermeier added a commit that referenced this issue Jan 15, 2020
If we encounter a list result that contains relationships
and nodes from a pattern comprehension both types of object will get
bundled into the OGM result.
@michael-simons
Copy link
Collaborator

Yep.

michael-simons added a commit that referenced this issue Mar 25, 2022
This fixes #902 in a similar fashion to what
0ba80b1 did for #821.

While #821 was fixed for non-domain objects as the `handle` method of
the `RestModelMapper.ResultRowBuilder` calls `convertToTargetContainer
which itself works recursively on non-domain objects, such an implicit
fix wasn't applied to domain objects.

As the domain objects are all flattened into the in-memory graph model
the hierarchy (parents and siblings) of nested lists must be stored
inside the `RestModelMapper`. This is achivied by a `Map<String,
List<String>>`, basically a nested list of pointers.

This map is used to recreate the shape of the original query.

This had some affects on the `SingleUseEntityMapper`: It may run into a
list of already mapped objects now, so those need some special
treatment, similar to queries such as mentioned in #737 (especially
queries containing and returning pattern comprehension without
extracting nodes and relationsships): They will work as before and
contain the mapped objects, but in the same fashion the pattern
comprehension produced them: In a list.
@michael-simons
Copy link
Collaborator

Hello @steffen-harbich-cognitum Here's a heads up for you: in #902 it was discovered that we flatten lists of known objects which shouldn't be the case as we don't do this for non-domain objects.

We are gonna fix that, but for your example it means a breaking change:

MATCH (n:Movie{title:'Pulp Fiction'}) return n, [(n)-[r:UNKNOWN]-(p) | [r,p]] as relAndNode

with the array around it, it will map to a List of node and relationships as it is actually correct (that's what a pattern comprehension produces).

A workaround to not change your calling code will be

MATCH (n:Movie{title:'Pulp Fiction'}) WITH n, [(n)-[r:UNKNOWN]-(p) | [r,p]] as patterns UNWIND patterns as relAndNode RETURN n, relAndNode

the mapping itself won't change.

Otherwise, the calling / usage side need a bit of change: It must now get the first index of the created pattern. See

@Test // GH-737
public void shouldReturnListOfNodesAndRelationshipModelForUnknownRelationshipLists() {
Result result = session
.query("MATCH (n:Movie{title:'Pulp Fiction'}) return n, [(n)-[r:UNKNOWN]-(p) | [r,p]] as relAndNode",
emptyMap());
Map<String, Object> returnedRow = result.queryResults().iterator().next();
assertThat(returnedRow.get("n")).isInstanceOf(Movie.class);
assertThat(returnedRow.get("relAndNode")).isInstanceOf(List.class);
List<?> relAndNode = ((List<List<?>>) returnedRow.get("relAndNode")).get(0);
assertThat(relAndNode)
.hasSize(2)
.satisfies(v -> assertThat(v).isInstanceOf(Pet.class), Index.atIndex(0))
.satisfies(v -> assertThat(v).isInstanceOf(RelationshipModel.class), Index.atIndex(1));
}

Thank you and sorry for any possible inconvenience.

michael-simons added a commit that referenced this issue Mar 25, 2022
This fixes #902 in a similar fashion to what
0ba80b1 did for #821.

While #821 was fixed for non-domain objects as the `handle` method of
the `RestModelMapper.ResultRowBuilder` calls `convertToTargetContainer
which itself works recursively on non-domain objects, such an implicit
fix wasn't applied to domain objects.

As the domain objects are all flattened into the in-memory graph model
the hierarchy (parents and siblings) of nested lists must be stored
inside the `RestModelMapper`. This is achivied by a `Map<String,
List<String>>`, basically a nested list of pointers.

This map is used to recreate the shape of the original query.

This had some affects on the `SingleUseEntityMapper`: It may run into a
list of already mapped objects now, so those need some special
treatment, similar to queries such as mentioned in #737 (especially
queries containing and returning pattern comprehension without
extracting nodes and relationsships): They will work as before and
contain the mapped objects, but in the same fashion the pattern
comprehension produced them: In a list.
@steffen-harbich-cognitum
Copy link
Author

Alright, thanks for info. I will keep that in mind when we update.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants