-
Notifications
You must be signed in to change notification settings - Fork 49
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
Adding support for list type property filters #285
Conversation
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.
Also update the schema-tests, they are currently failing.
@@ -39,6 +39,10 @@ enum class FieldOperator( | |||
EW("_ends_with", { lhs, rhs -> lhs.endsWith(rhs) }), | |||
MATCHES("_matches", { lhs, rhs -> lhs.matches(rhs) }), | |||
|
|||
EQ_LIST("", { lhs, rhs -> lhs.isEqualTo(rhs) }, list = true), |
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.
The JS-API already defines the following fields:
- test: [String!]
- test_NOT: [String!]
- test_INCLUDES: String
- test_NOT_INCLUDES: String
So we should not create conflicting definitions here, since we are on the way to align our API with the one generated by the JS-version.
So the new fields should be named as follows after coordination with the JS library:
- test_INCLUDES_SOME: [String!]
- test_INCLUDES_ALL: [String!]
- test_INCLUDES_NONE: [String!]
- test_INCLUDES_SINGLE: [String!]
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.
Yes this approach looks good, I will make the changes according to this. And I think subsequent changes will be required in cypher-dsl
library also for these new fields, so I will check that also parallelly. Thank you.
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.
There is no need to update cypher-dsl
for this, you can just use the predicates directly, like:
INCLUDES_ALL("_includes_all", { lhs, rhs ->
val x: SymbolicName = org.neo4j.cypherdsl.core.Cypher.name("x")
Predicates.all(x).`in`(lhs).where(x.`in`(rhs))
}, list = true),
@@ -72,6 +75,131 @@ CREATE (p:Person)-[:WORKS_AT]->(c) | |||
SET p = props | |||
---- | |||
|
|||
== Filter on _List_ |
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.
add Test for all new predicates (I miss the hobbies
-field test)
Following new definitions are added
Test cases added for schema augmentation and cypher generation |
This PR adds support for filters which can be used with list type properties of Node or Relation.
Filters added are:
INCLUDES_ANY
,INCLUDES_ALL
,NOT_INCLUDES
.This PR is in reference with the PR of cypher-dsl library (neo4j/cypher-dsl#374) which includes enhancement for list filters in that library.
Test cases are included for all the code changes implemented.