Skip to content
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

bugfix: arguments with null default were required #509

Merged
merged 9 commits into from
Apr 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,3 +69,4 @@
- feature: improved descriptions for all internal error states

## master
- bugfix: function arguments with a null default value were required instead of optional
62 changes: 61 additions & 1 deletion docs/functions.md
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ Functions accepting or returning arrays of non-composite types are also supporte

## Default Arguments

Functions with default arguments can have their default arguments omitted.
Arguments without a default value are required in the GraphQL schema, to make them optional they should have a default value.

=== "Function"

Expand Down Expand Up @@ -410,6 +410,66 @@ Functions with default arguments can have their default arguments omitted.
}
```

If there is no sensible default, and you still want to make the argument optional, consider using the default value null.


=== "Function"

```sql
create function "addNums"(a int default null, b int default null)
returns int
immutable
language plpgsql
as $$
begin

if a is null and b is null then
raise exception 'a and b both can''t be null';
end if;

if a is null then
return b;
end if;

if b is null then
return a;
end if;

return a + b;
end;
$$;
```

=== "QueryType"

```graphql
type Query {
addNums(a: Int, b: Int): Int
}
```


=== "Query"

```graphql
query {
addNums(a: 42)
}
```

=== "Response"

```json
{
"data": {
"addNums": 42
}
}
```

Currently, null defaults are only supported as simple expressions, as shown in the previous example.


## Limitations

The following features are not yet supported. Any function using these features is not exposed in the API:
Expand Down
22 changes: 16 additions & 6 deletions src/graphql.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1373,12 +1373,22 @@ fn function_args(schema: &Arc<__Schema>, func: &Arc<Function>) -> Vec<__InputVal
(t, arg_name, arg_default)
})
})
.map(|(arg_type, arg_name, arg_default)| __InputValue {
name_: schema.graphql_function_arg_name(func, arg_name),
type_: arg_type,
description: None,
default_value: arg_default,
sql_type: None,
.map(|(arg_type, arg_name, arg_default)| {
let default_value = if let Some(default_value) = arg_default {
match default_value {
DefaultValue::NonNull(value) => Some(value),
DefaultValue::Null => None,
}
} else {
None
};
__InputValue {
name_: schema.graphql_function_arg_name(func, arg_name),
type_: arg_type,
description: None,
default_value,
sql_type: None,
}
})
.collect()
}
Expand Down
44 changes: 30 additions & 14 deletions src/sql_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ pub struct Function {
}

impl Function {
pub fn args(&self) -> impl Iterator<Item = (u32, &str, Option<&str>, Option<String>)> {
pub fn args(&self) -> impl Iterator<Item = (u32, &str, Option<&str>, Option<DefaultValue>)> {
ArgsIterator::new(
&self.arg_types,
&self.arg_type_names,
Expand Down Expand Up @@ -200,7 +200,13 @@ struct ArgsIterator<'a> {
arg_types: &'a [u32],
arg_type_names: &'a Vec<String>,
arg_names: &'a Option<Vec<String>>,
arg_defaults: Vec<Option<String>>,
arg_defaults: Vec<Option<DefaultValue>>,
}

#[derive(Clone)]
pub(crate) enum DefaultValue {
NonNull(String),
Null,
}

impl<'a> ArgsIterator<'a> {
Expand Down Expand Up @@ -230,7 +236,7 @@ impl<'a> ArgsIterator<'a> {
arg_defaults: &'a Option<String>,
num_default_args: usize,
num_total_args: usize,
) -> Vec<Option<String>> {
) -> Vec<Option<DefaultValue>> {
let mut defaults = vec![None; num_total_args];
let Some(arg_defaults) = arg_defaults else {
return defaults;
Expand All @@ -249,23 +255,33 @@ impl<'a> ArgsIterator<'a> {
debug_assert!(num_default_args <= num_total_args);
let start_idx = num_total_args - num_default_args;
for i in start_idx..num_total_args {
defaults[i] =
Self::sql_to_graphql_default(default_strs[i - start_idx], arg_types[i])
defaults[i] = Self::sql_to_graphql_default(default_strs[i - start_idx], arg_types[i])
}

defaults
}

fn sql_to_graphql_default(default_str: &str, type_oid: u32) -> Option<String> {
fn sql_to_graphql_default(default_str: &str, type_oid: u32) -> Option<DefaultValue> {
let trimmed = default_str.trim();
if trimmed.starts_with("NULL::") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are you confident this is always reliable wrt uppercase/lowercase? Will it alway be cast with a ::?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are you confident this is always reliable wrt uppercase/lowercase?

Yes, Postgres converts all cases into capitals. Added a test for this.

Will it alway be cast with a ::?

A <arg> <type> default null form is always converted into a cast of the form NULL::<type>. Other, more complex forms of expressions might not work.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also updated docs with the caveat that only simple default null are supported for now.

return Some(DefaultValue::Null);
}
match type_oid {
21 | 23 => trimmed.parse::<i32>().ok().map(|i| i.to_string()),
16 => trimmed.parse::<bool>().ok().map(|i| i.to_string()),
700 | 701 => trimmed.parse::<f64>().ok().map(|i| i.to_string()),
25 => trimmed
.strip_suffix("::text")
.to_owned()
.map(|i| format!("\"{}\"", i.trim_matches(',').trim_matches('\''))),
21 | 23 => trimmed
.parse::<i32>()
.ok()
.map(|i| DefaultValue::NonNull(i.to_string())),
16 => trimmed
.parse::<bool>()
.ok()
.map(|i| DefaultValue::NonNull(i.to_string())),
700 | 701 => trimmed
.parse::<f64>()
.ok()
.map(|i| DefaultValue::NonNull(i.to_string())),
25 => trimmed.strip_suffix("::text").map(|i| {
DefaultValue::NonNull(format!("\"{}\"", i.trim_matches(',').trim_matches('\'')))
}),
_ => None,
}
}
Expand All @@ -276,7 +292,7 @@ lazy_static! {
}

impl<'a> Iterator for ArgsIterator<'a> {
type Item = (u32, &'a str, Option<&'a str>, Option<String>);
type Item = (u32, &'a str, Option<&'a str>, Option<DefaultValue>);

fn next(&mut self) -> Option<Self::Item> {
if self.index < self.arg_types.len() {
Expand Down
Loading
Loading