-
Notifications
You must be signed in to change notification settings - Fork 165
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
fix: Create/Drop a single/composite index for relationships #948
Conversation
@meistermeier Hi Gerrit Meier, can you help me to look at this PR, and release a 3.2.40 version? |
Thanks for the PR. I have seen this already and it's definitely on my list. |
Sorry for the delay. I have still not found the time to have a more detailed look. |
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.
Thanks for the PR (again). I think that we can adapt this, if you change some minor bits.
FYI: We discarded the whole index functionality with Neo4j-OGM 4.0
You also need to sign CLA, so we can merge your code. https://neo4j.com/developer/cla/
core/src/main/java/org/neo4j/ogm/autoindex/AutoIndexManager.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/neo4j/ogm/autoindex/AutoIndexManager.java
Outdated
Show resolved
Hide resolved
Thanks for your review, I have modified the code following the points you mentioned,please check it. |
@@ -287,7 +299,9 @@ private static Set<AutoIndex> initialiseAutoIndex(MetaData metaData) { | |||
|
|||
for (FieldInfo fieldInfo : getIndexFields(classInfo)) { | |||
|
|||
IndexType type = fieldInfo.isConstraint() ? IndexType.UNIQUE_CONSTRAINT : IndexType.SINGLE_INDEX; | |||
IndexType type = fieldInfo.isConstraint() ? |
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.
Sorry, I missed this bit for the ternary operation clean up. Would be good that it gets the same format as the one, you just fixed.
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.
Thanks, It's my carelessness. I have fixed it.
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.
Happens to be all the time 😄
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.
so could you merge this PR now?
Done, thanks again. |
Released with 3.2.41: https://github.com/neo4j/neo4j-ogm/releases/tag/v3.2.41 |
Since neo4j supports relationship index in versions above 4.x , it is necessary to distinguish node and relationship index in IndexType.