-
Notifications
You must be signed in to change notification settings - Fork 148
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
Generated *OnCreateInput
types do not include fields to connect/create types with @relationship
directives
#5250
Comments
Many thanks for raising this bug report @andreloeffelmann. 🐛 We will now attempt to reproduce the bug based on the steps you have provided. Please ensure that you've provided the necessary information for a minimal reproduction, including but not limited to:
If you have a support agreement with Neo4j, please link this GitHub issue to a new or existing Zendesk ticket. Thanks again! 🙏 |
Corresponding Zendesk ticket: here |
We've been able to confirm this bug using the steps to reproduce that you provided - many thanks @andreloeffelmann! 🙏 We will now prioritise the bug and address it appropriately. |
@a-alle, why has this been re-labelled as |
Hi @andreloeffelmann, I appreciate the confusion here. This is more of a case of "there's nothing for us to do" - this is a shortcoming of Cypher In version 6.0.0 of the Neo4j GraphQL Library we're going to be making some drastic changes, including removing the ability to model 1:1 relationships as in this bug report - all relationships will have to be modelled as lists. Fundamentally, we've been battling the database for the past 3 years, and it's time to have the library only support features which the database itself natively supports. Until the database supports full schema including relationship cardinality, this will not be supported in the library, to avoid these kinds of issues. |
Hi @darrellwarde, thanks for your quick reply! Are you saying that :1 relationships or 1:1 relationships won't be supported with v6.? Example for :1 relation: type Product {
id: Int
price: Price! @relationship(type: "PRODUCT_HAS_PRICE", direction: OUT, aggregate:false)
}
type Price {
currency: String!
value: Int!
} Example for 1:1 relation: type Product {
id: Int
price: Price! @relationship(type: "PRODUCT_HAS_PRICE", direction: OUT, aggregate:false)
}
type Price {
currency: String!
value: Int!
product: Product! @relationship(type: "PRODUCT_HAS_PRICE", direction: IN, aggregate:false)
} Which kind of type definitions will NOT be supported with v6? This is a huge thing. Our database and graphql API model is partially based on these kinds of type definitions. |
Another thought on this: type Product {
id: Int
price: Price! @cypher(statement: """
MATCH (this)-[:PRODUCT_HAS_PRICE]->(p:Price) RETURN {"currency":p.currency, "value":p.value} AS result
""" columnName: "result")
}
type Price {
currency: String!
value: Int!
} But in that case it won't be possible to filter on the Price of a Product, like "give me all Products having a Price greater than X" since we have used a |
You are quite possibly right that we should remove it - it would actually be a removal of the entire As to what won't be possible in version 6.0.0 (or whatever major version it ends up being when we release these broad API changes) - both of the examples you have given will not be supported, essentially any non-list relationship. I know it might seem somewhat nonsensical to remove such "basic" functionality, but due to the lack of any enforcement of relationship cardinality in the database, we have been forced down the route of manual cardinality checks in our generated Cypher which are not comprehensive and poorly performing.
Yes! We are scheduled to work on this feature in the coming months to be released in the current major version of the library, so this could be the workaround (for read scenarios, anyway). |
I think that would be a good solution! Don't get me wrong here - you guys really are doing a great job. The API is a great tool to work with and works like a charm (in most cases ;-) )
Glad to here that :-) That's really good news! However, what about write scenarios? How should one model such an object-subobject thing? I think this is a quite common requirement for data models. Not in every case has an object a list of sub-objects. Sometimes it is good to structure data in a simple sub-object, just to group things. And what should be considered always: people do not only need to read from the API - they need to write too! So if you are saying this will be implemented for read-scenarios - how on earth should we write the Product-Price thing to the database? type Product {
id: Int
price_value: Int!
price_currency: String!
} But this is totally ridiculous and would blow-up the I hope you consider these problems for your next major release. Right now I don't see any possibility for us to go with the update and migrate our code. A such fundamental breaking change at data-model level without any possible workaround or alternative... delicate! Or am I missing something? Do you see a solution? |
Thanks for the kind words about the API! I completely agree about write scenarios - we had a conversation this morning but we have a lot to think about. There are options such as exposing these as types in GraphQL, but storing them as JSON serialized strings (or similar) in node properties, but then querying in any way aside from the GraphQL API becomes clunky. We very much appreciate that this could be quite the jarring migration, which is why we will maintain version 5.x as an LTS release. Hopefully we can influence some changes in the database which will make these changes easier! |
Type definitions
Test data
No response
Steps to reproduce
What happened
The generated input type
MouseOnCreateInput
misses the fieldeats: MouseEatsFieldInput!
and looks like this:Thus, it is possible to execute the following mutation:
This mutation creates an inconsistent node
Mouse
inside the database missing the required relationMOUSE_EATS_CHEESE
to aCheese
node!Expected behaviour
The generated input type
MouseOnCreateInput
should contain the fieldeats: MouseEatsFieldInput!
, just asMouseCreateInput
, looking like this:Version
5.4.3
Database version
5.20.0
Relevant log output
No response
The text was updated successfully, but these errors were encountered: