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

[rGkwaqu1] apoc.path.expand doesn't handle special characters in label name #415

Merged
merged 2 commits into from
Jun 19, 2023

Conversation

vga91
Copy link
Collaborator

@vga91 vga91 commented May 19, 2023

Unlike what is written in the card,
the issue regards the : character, which handle compound labels.


In this pr I updated the : char so that an escape character \ can be inserted.
In case it's ok, the documentation should be updated.

Otherwise, we could just explain better that the : char
is used to handle multiple labels and that it is not possible to filter labels containing that character.

@Lojjs Lojjs self-assigned this May 24, 2023
Copy link
Contributor

@Lojjs Lojjs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the late review. The solution looks OK, but I think it would be good with better test coverage

core/src/test/java/apoc/path/ExpandPathTest.java Outdated Show resolved Hide resolved
@Test
public void testLabelWithSpecialChar() {
db.executeTransactionally(
"CREATE (n:`http://example.com/abc#Object` {one: 'alpha'})-[:REL]->(o:`http://www.w3.org/2002/07/owl#Class`:OwlClass {two: 'beta'})"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would write the test setup slightly different so there are some nodes with e.g. the labels
http + //example.com/abc#Object, only http or other similar cases that should be filtered out by the label filter. That way it would be easier to notice if there are any bugs around this solution.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added

Copy link
Contributor

@Lojjs Lojjs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing

@vga91 vga91 force-pushed the apoc.path.expand-twodots-chars branch from 073a3ff to ce7677d Compare June 19, 2023 10:05
@vga91 vga91 merged commit 4d25153 into dev Jun 19, 2023
@vga91 vga91 deleted the apoc.path.expand-twodots-chars branch June 19, 2023 14:12
@vga91
Copy link
Collaborator Author

vga91 commented Jun 19, 2023

CI error fixed here

vga91 added a commit to neo4j-contrib/neo4j-apoc-procedures that referenced this pull request Jun 19, 2023
…l name (neo4j/apoc#415)

* [rGkwaqu1] apoc.path.expand doesn't handle special characters in label name

* [rGkwaqu1] added tests
vga91 added a commit to neo4j-contrib/neo4j-apoc-procedures that referenced this pull request Jun 20, 2023
…l name (neo4j/apoc#415)

* [rGkwaqu1] apoc.path.expand doesn't handle special characters in label name

* [rGkwaqu1] added tests
vga91 added a commit to neo4j-contrib/neo4j-apoc-procedures that referenced this pull request Jun 20, 2023
…l name (neo4j/apoc#415) (#3633)

* [rGkwaqu1] apoc.path.expand doesn't handle special characters in label name

* [rGkwaqu1] added tests
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

Successfully merging this pull request may close these issues.

2 participants