Skip to content

fix: mixing directed and undirected edges in motif throws parse error #754

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.

Sign up for GitHub

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

Jump to bottom
Open
goungoun wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix: mixing directed and undirected edges in motif throws parse error #754

goungoun wants to merge 3 commits into from

Conversation

@goungoun
Copy link
Contributor

@goungoun goungoun commented Nov 22, 2025 •
edited
Loading

What changes were proposed in this pull request?

This PR address the bug report #750
(u)-[e1]-(v);(v)-[e2]->(k) should not fail

Why are the changes needed?

It improves the flexibility to use the motif pattern

Test Case

image
  1. (u)-[e1]-(v)
> g.find("(u)-[e1]-(v)").where("u.id == 0").select("u.id", "v.id", "_pattern", "_direction").show()
+---+---+-------------+----------+
| id| id|     _pattern|_direction|
+---+---+-------------+----------+
|  0|  1|(u)<-[e1]-(v)|        in|
|  0|  2|(u)<-[e1]-(v)|        in|
|  0|  1|(u)-[e1]->(v)|       out|
+---+---+-------------+----------+
  1. (u)-[e1]-(v);(v)-[e2]->(k)
> g.find("(u)-[e1]-(v);(v)-[e2]->(k)").where("u.id == 0").select("u.id", "v.id", "_pattern","_direction", "k.id").show()
+---+---+-------------+----------+---+
| id| id|     _pattern|_direction| id|
+---+---+-------------+----------+---+
|  0|  1|(u)<-[e1]-(v)|        in|  2|
|  0|  1|(u)<-[e1]-(v)|        in|  0|
|  0|  2|(u)<-[e1]-(v)|        in|  0|
|  0|  2|(u)<-[e1]-(v)|        in|  3|
|  0|  1|(u)-[e1]->(v)|       out|  2|
|  0|  1|(u)-[e1]->(v)|       out|  0|
+---+---+-------------+----------+---+

@goungoun goungoun mentioned this pull request Nov 22, 2025
Open
6 tasks
@goungoun goungoun changed the title [bug] Mixing directed and undirected edges in motif throws parse error [draft] Mixing directed and undirected edges in motif throws parse error Nov 22, 2025
@rjurney
Copy link
Collaborator

rjurney commented Nov 22, 2025

What about 2, 3 in the first example?

@rjurney
Copy link
Collaborator

rjurney commented Nov 22, 2025

0, 1, 2 repeats

@goungoun
Copy link
Contributor Author

goungoun commented Nov 23, 2025

@rjurney It was with the condition "u.id == 0", my mistake in the description. Thanks!

@goungoun
Copy link
Contributor Author

goungoun commented Nov 23, 2025

The last commit removes the existing undirected pattern based on a regular expression, as the new one in the findIncremental() method can replace it.

@SemyonSinchenko
Copy link
Collaborator

SemyonSinchenko commented Nov 26, 2025

It looks ready for review, isn't it?

@goungoun
Copy link
Contributor Author

goungoun commented Nov 26, 2025

It looks ready for review, isn't it?

Let me keep it draft this week.

@goungoun goungoun changed the title [draft] Mixing directed and undirected edges in motif throws parse error Mixing directed and undirected edges in motif throws parse error Nov 30, 2025
@goungoun goungoun changed the title Mixing directed and undirected edges in motif throws parse error [bug] Mixing directed and undirected edges in motif throws parse error Nov 30, 2025
@goungoun goungoun changed the title [bug] Mixing directed and undirected edges in motif throws parse error [fix] Mixing directed and undirected edges in motif throws parse error Nov 30, 2025
@goungoun goungoun changed the title [fix] Mixing directed and undirected edges in motif throws parse error fix: mixing directed and undirected edges in motif throws parse error Nov 30, 2025
@goungoun goungoun marked this pull request as ready for review November 30, 2025 08:45
@goungoun
Copy link
Contributor Author

goungoun commented Dec 2, 2025 •
edited
Loading

0, 1, 2 repeats
@rjurney I added two columns, "_pattern","_direction" in the example to explain the symptom.

@goungoun
Copy link
Contributor Author

goungoun commented Dec 3, 2025

@rjurney The result is as expected, is there anything you're still unsure about?

@SemyonSinchenko SemyonSinchenko self-requested a review December 3, 2025 06:57
@SemyonSinchenko
Copy link
Collaborator

SemyonSinchenko commented Dec 9, 2025

@goungoun Hello! Sorry for the delay with a review. I want to finish my work on graph embeddings first and I'm almost done... I will try to review your PR this week. Thanks for understanding!

@goungoun
Copy link
Contributor Author

goungoun commented Dec 9, 2025

@goungoun Hello! Sorry for the delay with a review. I want to finish my work on graph embeddings first and I'm almost done... I will try to review your PR this week. Thanks for understanding!

@SemyonSinchenko Thanks for your comment. I know you are running many things, please no rush as I can wait.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SemyonSinchenko SemyonSinchenko Awaiting requested review from SemyonSinchenko

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@goungoun @rjurney @SemyonSinchenko