From 3e2628bd8d863c924b9fd787f9913e04b327e849 Mon Sep 17 00:00:00 2001 From: Raminder Singh Date: Tue, 25 Jun 2024 17:12:11 +0530 Subject: [PATCH 1/2] fix #533 in which a default arg was marked as required --- src/sql_types.rs | 14 +- test/expected/issue_533.out | 382 ++++++++++++++++++++++++++++++++++++ test/sql/issue_533.sql | 159 +++++++++++++++ 3 files changed, 554 insertions(+), 1 deletion(-) create mode 100644 test/expected/issue_533.out create mode 100644 test/sql/issue_533.sql diff --git a/src/sql_types.rs b/src/sql_types.rs index e5e84c0f..a04a7dee 100644 --- a/src/sql_types.rs +++ b/src/sql_types.rs @@ -263,10 +263,12 @@ impl<'a> ArgsIterator<'a> { fn sql_to_graphql_default(default_str: &str, type_oid: u32) -> Option { let trimmed = default_str.trim(); + if trimmed.starts_with("NULL::") { return Some(DefaultValue::Null); } - match type_oid { + + let res = match type_oid { 21 | 23 => trimmed .parse::() .ok() @@ -283,6 +285,16 @@ impl<'a> ArgsIterator<'a> { DefaultValue::NonNull(format!("\"{}\"", i.trim_matches(',').trim_matches('\''))) }), _ => None, + }; + + // return the non-parsed value as default if for whatever reason the default value can't + // be parsed into a value of the required type. This fixes problems where the default + // is a complex expression like a function call etc. See test/sql/issue_533.sql for + // a test case for this scenario. + if res.is_some() { + res + } else { + Some(DefaultValue::NonNull(trimmed.to_string())) } } } diff --git a/test/expected/issue_533.out b/test/expected/issue_533.out new file mode 100644 index 00000000..7dc20a1c --- /dev/null +++ b/test/expected/issue_533.out @@ -0,0 +1,382 @@ +begin; + savepoint a; + create function public.uid() + returns uuid stable language sql as + $$ + select 'adfc9433-b00e-4072-bbe5-5ae23a76e15d'::uuid + $$; + create function public.get_user_id(p_user_id uuid default public.uid()) + returns uuid stable language sql as + $$ + select p_user_id; + $$; + select jsonb_pretty(graphql.resolve($$ + query IntrospectionQuery { + __schema { + queryType { + fields { + name + description + type { + kind + } + args { + name + defaultValue + type { + name + kind + ofType { + name + kind + } + } + } + } + } + } + } $$)); + jsonb_pretty +------------------------------------------------------------------------ + { + + "data": { + + "__schema": { + + "queryType": { + + "fields": [ + + { + + "args": [ + + { + + "name": "pUserId", + + "type": { + + "kind": "SCALAR", + + "name": "UUID", + + "ofType": null + + }, + + "defaultValue": "uid()" + + } + + ], + + "name": "getUserId", + + "type": { + + "kind": "SCALAR" + + }, + + "description": null + + }, + + { + + "args": [ + + { + + "name": "nodeId", + + "type": { + + "kind": "NON_NULL", + + "name": null, + + "ofType": { + + "kind": "SCALAR", + + "name": "ID" + + } + + }, + + "defaultValue": null + + } + + ], + + "name": "node", + + "type": { + + "kind": "INTERFACE" + + }, + + "description": "Retrieve a record by its `ID`"+ + }, + + { + + "args": [ + + ], + + "name": "uid", + + "type": { + + "kind": "SCALAR" + + }, + + "description": null + + } + + ] + + } + + } + + } + + } +(1 row) + + select jsonb_pretty(graphql.resolve($$ + query { + userId: getUserId( + pUserId: "34f45987-bb41-4111-967c-bd462ea41d52" + ) + } + $$)); + jsonb_pretty +---------------------------------------------------------- + { + + "data": { + + "userId": "34f45987-bb41-4111-967c-bd462ea41d52"+ + } + + } +(1 row) + + select jsonb_pretty(graphql.resolve($$ + query { + userId: getUserId + } + $$)); + jsonb_pretty +---------------------------------------------------------- + { + + "data": { + + "userId": "adfc9433-b00e-4072-bbe5-5ae23a76e15d"+ + } + + } +(1 row) + + rollback to savepoint a; + create function public.uid() + returns text stable language sql as + $$ + select 'adfc9433-b00e-4072-bbe5-5ae23a76e15d' + $$; + create function public.get_user_id(p_user_id text default public.uid()) + returns text stable language sql as + $$ + select p_user_id; + $$; + select jsonb_pretty(graphql.resolve($$ + query IntrospectionQuery { + __schema { + queryType { + fields { + name + description + type { + kind + } + args { + name + defaultValue + type { + name + kind + ofType { + name + kind + } + } + } + } + } + } + } $$)); + jsonb_pretty +------------------------------------------------------------------------ + { + + "data": { + + "__schema": { + + "queryType": { + + "fields": [ + + { + + "args": [ + + { + + "name": "pUserId", + + "type": { + + "kind": "SCALAR", + + "name": "String", + + "ofType": null + + }, + + "defaultValue": "uid()" + + } + + ], + + "name": "getUserId", + + "type": { + + "kind": "SCALAR" + + }, + + "description": null + + }, + + { + + "args": [ + + { + + "name": "nodeId", + + "type": { + + "kind": "NON_NULL", + + "name": null, + + "ofType": { + + "kind": "SCALAR", + + "name": "ID" + + } + + }, + + "defaultValue": null + + } + + ], + + "name": "node", + + "type": { + + "kind": "INTERFACE" + + }, + + "description": "Retrieve a record by its `ID`"+ + }, + + { + + "args": [ + + ], + + "name": "uid", + + "type": { + + "kind": "SCALAR" + + }, + + "description": null + + } + + ] + + } + + } + + } + + } +(1 row) + + select jsonb_pretty(graphql.resolve($$ + query { + userId: getUserId( + pUserId: "34f45987-bb41-4111-967c-bd462ea41d52" + ) + } + $$)); + jsonb_pretty +---------------------------------------------------------- + { + + "data": { + + "userId": "34f45987-bb41-4111-967c-bd462ea41d52"+ + } + + } +(1 row) + + select jsonb_pretty(graphql.resolve($$ + query { + userId: getUserId + } + $$)); + jsonb_pretty +---------------------------------------------------------- + { + + "data": { + + "userId": "adfc9433-b00e-4072-bbe5-5ae23a76e15d"+ + } + + } +(1 row) + + rollback to savepoint a; + create function public.add_nums(a int default 1 + 1, b int default 2 + 2) + returns int stable language sql as + $$ + select a + b; + $$; + select jsonb_pretty(graphql.resolve($$ + query IntrospectionQuery { + __schema { + queryType { + fields { + name + description + type { + kind + } + args { + name + defaultValue + type { + name + kind + ofType { + name + kind + } + } + } + } + } + } + } $$)); + jsonb_pretty +------------------------------------------------------------------------ + { + + "data": { + + "__schema": { + + "queryType": { + + "fields": [ + + { + + "args": [ + + { + + "name": "a", + + "type": { + + "kind": "SCALAR", + + "name": "Int", + + "ofType": null + + }, + + "defaultValue": "(1 + 1)" + + }, + + { + + "name": "b", + + "type": { + + "kind": "SCALAR", + + "name": "Int", + + "ofType": null + + }, + + "defaultValue": "(2 + 2)" + + } + + ], + + "name": "addNums", + + "type": { + + "kind": "SCALAR" + + }, + + "description": null + + }, + + { + + "args": [ + + { + + "name": "nodeId", + + "type": { + + "kind": "NON_NULL", + + "name": null, + + "ofType": { + + "kind": "SCALAR", + + "name": "ID" + + } + + }, + + "defaultValue": null + + } + + ], + + "name": "node", + + "type": { + + "kind": "INTERFACE" + + }, + + "description": "Retrieve a record by its `ID`"+ + } + + ] + + } + + } + + } + + } +(1 row) + + select jsonb_pretty(graphql.resolve($$ + query { + addNums(a: 1, b: 2) + } + $$)); + jsonb_pretty +---------------------- + { + + "data": { + + "addNums": 3+ + } + + } +(1 row) + + select jsonb_pretty(graphql.resolve($$ + query { + addNums + } + $$)); + jsonb_pretty +---------------------- + { + + "data": { + + "addNums": 6+ + } + + } +(1 row) + +rollback; diff --git a/test/sql/issue_533.sql b/test/sql/issue_533.sql new file mode 100644 index 00000000..45070e2e --- /dev/null +++ b/test/sql/issue_533.sql @@ -0,0 +1,159 @@ +begin; + + savepoint a; + + create function public.uid() + returns uuid stable language sql as + $$ + select 'adfc9433-b00e-4072-bbe5-5ae23a76e15d'::uuid + $$; + + create function public.get_user_id(p_user_id uuid default public.uid()) + returns uuid stable language sql as + $$ + select p_user_id; + $$; + + select jsonb_pretty(graphql.resolve($$ + query IntrospectionQuery { + __schema { + queryType { + fields { + name + description + type { + kind + } + args { + name + defaultValue + type { + name + kind + ofType { + name + kind + } + } + } + } + } + } + } $$)); + + select jsonb_pretty(graphql.resolve($$ + query { + userId: getUserId( + pUserId: "34f45987-bb41-4111-967c-bd462ea41d52" + ) + } + $$)); + + select jsonb_pretty(graphql.resolve($$ + query { + userId: getUserId + } + $$)); + + rollback to savepoint a; + + create function public.uid() + returns text stable language sql as + $$ + select 'adfc9433-b00e-4072-bbe5-5ae23a76e15d' + $$; + + create function public.get_user_id(p_user_id text default public.uid()) + returns text stable language sql as + $$ + select p_user_id; + $$; + + select jsonb_pretty(graphql.resolve($$ + query IntrospectionQuery { + __schema { + queryType { + fields { + name + description + type { + kind + } + args { + name + defaultValue + type { + name + kind + ofType { + name + kind + } + } + } + } + } + } + } $$)); + + select jsonb_pretty(graphql.resolve($$ + query { + userId: getUserId( + pUserId: "34f45987-bb41-4111-967c-bd462ea41d52" + ) + } + $$)); + + select jsonb_pretty(graphql.resolve($$ + query { + userId: getUserId + } + $$)); + + rollback to savepoint a; + + create function public.add_nums(a int default 1 + 1, b int default 2 + 2) + returns int stable language sql as + $$ + select a + b; + $$; + + select jsonb_pretty(graphql.resolve($$ + query IntrospectionQuery { + __schema { + queryType { + fields { + name + description + type { + kind + } + args { + name + defaultValue + type { + name + kind + ofType { + name + kind + } + } + } + } + } + } + } $$)); + + select jsonb_pretty(graphql.resolve($$ + query { + addNums(a: 1, b: 2) + } + $$)); + + select jsonb_pretty(graphql.resolve($$ + query { + addNums + } + $$)); +rollback; From d99953610ecac48ba9fad2bdadf08659cacf8d5d Mon Sep 17 00:00:00 2001 From: Raminder Singh Date: Thu, 27 Jun 2024 13:21:04 +0530 Subject: [PATCH 2/2] return null defaultValue for complex default expressions --- docs/changelog.md | 2 +- src/sql_types.rs | 2 +- test/expected/issue_533.out | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/changelog.md b/docs/changelog.md index 57cd920f..d578aaf8 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -84,5 +84,5 @@ ## 1.5.6 - feature: add support for filtering on array column types using `contains`, `containedBy`, `overlaps`, `is`, `eq` - ## master +- bugfix: UDF argument with a complex default expression was marked as required diff --git a/src/sql_types.rs b/src/sql_types.rs index a04a7dee..1b367c66 100644 --- a/src/sql_types.rs +++ b/src/sql_types.rs @@ -294,7 +294,7 @@ impl<'a> ArgsIterator<'a> { if res.is_some() { res } else { - Some(DefaultValue::NonNull(trimmed.to_string())) + Some(DefaultValue::Null) } } } diff --git a/test/expected/issue_533.out b/test/expected/issue_533.out index 7dc20a1c..aa9ee2d3 100644 --- a/test/expected/issue_533.out +++ b/test/expected/issue_533.out @@ -52,7 +52,7 @@ begin; "name": "UUID", + "ofType": null + }, + - "defaultValue": "uid()" + + "defaultValue": null + } + ], + "name": "getUserId", + @@ -181,7 +181,7 @@ begin; "name": "String", + "ofType": null + }, + - "defaultValue": "uid()" + + "defaultValue": null + } + ], + "name": "getUserId", + @@ -305,7 +305,7 @@ begin; "name": "Int", + "ofType": null + }, + - "defaultValue": "(1 + 1)" + + "defaultValue": null + }, + { + "name": "b", + @@ -314,7 +314,7 @@ begin; "name": "Int", + "ofType": null + }, + - "defaultValue": "(2 + 2)" + + "defaultValue": null + } + ], + "name": "addNums", +