-
Notifications
You must be signed in to change notification settings - Fork 619
Parameterize Node DynamicLabels #3034
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
Parameterize Node DynamicLabels #3034
Conversation
@garbybaby
garbybaby
commented
Aug 7, 2025
- You have read the Spring Data Neo4j contribution guidelines.
- You use the code formatters provided here and have them applied to your changes. Don’t submit any formatting related changes.
- You submit test cases (unit or integration tests) that back your changes.
- You added yourself as author in the headers of the classes you touched. Amend the date range in the Apache license header if needed. For new types, add the license header (copy from another file and set the current year only).
Signed-off-by: Garby Baby <garbybaby@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This still generates a non-parametrized query for @DynamicLabels.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oldLabels.toArray(new String[0]) -> Cypher.parameter("oldLabels")
newLabels.toArray(new String[0]) -> Cypher.parameter("newLabels")
Parameter names are to be added to Constants and populated to the query when running it.
Hej there. Thanks for the PR.
There are no tests, hence I might be wrong in my assumption that the change will require an additional set of additional labels next to the additional labels we already have, that you called dynamic labels?
If so, this is not the approach we would like to support.
SDN does support dynamic labels already on the mapping side of things. The goal must be to translate those to either
- what there is now, building a static query for all the old versions of neo4j
- or translating it to proper dynamic Cypher labels
For the latter we would of course be using Cypher DSL. Over there, we don't support dynamic label expressions yet.
Hence, I think we should add this there, and than rely on the configured dialect so that Cypher-DSL does the correct thing.
I created in issue for the Cypher-DSL work neo4j/cypher-dsl#1314
zakjan
commented
Sep 2, 2025
@michael-simons By dynamic labels we meant only labels managed by SDN in @DynamicLabels, no more. It's good if Cypher DSL can support both static and dynamic labels depending on the dialect selection, when the labels are provided from a dynamic array. Static schema labels should stay static as-is to be eligible for planning with indexes. Dynamic labels don't use indexes well (docs) but this should be fine given their non-schema purpose.
zakjan
commented
Sep 2, 2025
@michael-simons Would you mind if we close this PR and leave it for you to finish the implementation? Given the missing and upcoming support in Cypher DSL, it's apparent it's more complex then we thought :)
@zakjan I got that (wrt dynamic labels), and I do fully get behind that we should translate the SDN feature to the new Cypher feature. We should have done this before.
Wrt this PR, do as you see fit, I am fine with both keeping it open or doing a fresh. Happy to help you and your colleague to get PR in.