fix #533 in which a default arg was marked as required #534
Conversation
test/expected/issue_533.out
Outdated
| "name": "UUID", + | ||
| "ofType": null + | ||
| }, + | ||
| "defaultValue": "uid()" + |
There was a problem hiding this comment.
I don't think we want to show the SQL logic of the default value here
Would it make sense to make the field nullable and not display the default value. I think we've used that approach in the past?
There was a problem hiding this comment.
If I understood correctly, your concern is only about the default value being a SQL expression, because the field is already nullable, right? In that case, we can replace uid() here with an empty string or something like <omittted> to indicate the complex nature of the default expression. We should probably also update the docs to explain why default value is an empty string or <omitted>.
There was a problem hiding this comment.
concern ... default value being a SQL expression
yeah exactly. I think it should be nullable (already done) and the defaultValue should be NULL.
Thats how Postgraphile handles it (screenshot), so seems like a safe bet
What kind of change does this PR introduce?
Bug fix
What is the current behavior?
UDF with a complex expression as default value for an argument made the argument required.
What is the new behavior?
The argument is no longer required now and if it is omitted it will take on the default value. Complex default expressions like function calls or arithmetic expressions etc. will not be evaluated but will be included as is when returning them in response to introspection queries.
Additional context
Fixes #533