-
-
Notifications
You must be signed in to change notification settings - Fork 104
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
add support for calling UDFs with default arguments #427
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.
nice!
test/expected/function_calls.out
Outdated
"type": { + | ||
"name": "Int" + | ||
}, + | ||
"defaultValue": null+ |
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.
current the defaults do not render in the introspection schema
that's where IDEs like GraphiQL pull the info and do syntax highlighting from so we should support it there too
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 defaults can be extracted from the proargdefaults
column of the pg_proc
table. This column's type is pg_node_tree
which can be tricky to parse as it looks like this: ({CONST :consttype 16 :consttypmod -1 :constcollid 0 :constlen 1 :constbyval true :constisnull false :location 132 :constvalue 1 [ 0 0 0 0 0 0 0 0 ]})
.
Calling pg_get_expr
built-in function on a pg_node_tree
type returns a string representation which looks more manageable but can still be quite involved for complex values like json & jsonb.
Some example values for pg_get_expr(proargdefaults)
below:
postgres=# select proname, pronargs, pg_get_expr(proargdefaults, 0) from pg_catalog.pg_proc where proargdefaults is not null;
proname | pronargs | pg_get_expr
-------------------------------------+----------+------------------------------------
jsonb_insert | 4 | false
jsonb_path_query | 4 | '{}'::jsonb, false
jsonb_path_exists_tz | 4 | '{}'::jsonb, false
jsonb_path_query_array_tz | 4 | '{}'::jsonb, false
pg_logical_slot_peek_changes | 4 | '{}'::text[]
pg_logical_slot_peek_binary_changes | 4 | '{}'::text[]
pg_create_logical_replication_slot | 4 | false, false
is_normalized | 2 | 'NFC'::text
jsonb_set_lax | 5 | true, 'use_json_null'::text
parse_ident | 2 | true
pg_terminate_backend | 2 | 0
json_populate_record | 3 | false
json_populate_recordset | 3 | false
pg_logical_slot_get_changes | 4 | '{}'::text[]
pg_logical_slot_get_binary_changes | 4 | '{}'::text[]
pg_create_physical_replication_slot | 3 | false, false
make_interval | 7 | 0, 0, 0, 0, 0, 0, 0.0
jsonb_set | 4 | true
pg_backup_start | 2 | false
jsonb_path_exists | 4 | '{}'::jsonb, false
jsonb_path_match | 4 | '{}'::jsonb, false
jsonb_path_query_array | 4 | '{}'::jsonb, false
jsonb_path_query_first | 4 | '{}'::jsonb, false
jsonb_path_match_tz | 4 | '{}'::jsonb, false
jsonb_path_query_tz | 4 | '{}'::jsonb, false
jsonb_path_query_first_tz | 4 | '{}'::jsonb, false
normalize | 2 | 'NFC'::text
pg_backup_stop | 1 | true
pg_promote | 2 | true, 60
install_extension | 5 | NULL::text[]
concat_text | 2 | 'hello, hi'::text, 'ok, bye'::text
get_json_obj | 2 | '{"key": "value::json"}'::json
add_floats | 2 | ((1)::numeric + 3.14)
(33 rows)
Defaults for UDFs contact_text
, get_json_obj
and add_floats
above are create function concat_text(a text default 'hello, hi', b text default 'ok, bye')
, create function get_json_obj(key text, input json default '{ "key": "value::json" }')
and create function add_floats(a float, b float default (1 + 3.14))
respectively.
We can probably parse literal expressions but more complex default expressions will likely still fail.
Note that Postgraphile returns null for defaultValue
for function args.
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.
Note that Postgraphile returns null for defaultValue for function args.
Interesting, yeah, sounds like a hard problem to do perfectly.
It does seem like handling simple scalar cases like int/bigint/text/varchar/bool would make for easier to understand APIs but its a backwards compatible change to show the default value
does that scope seem more reasonable?
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.
I'll be OOO so will approve this. Up to you if you'd like to handle those scalar cases here or merge it & add that to #410
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.
Implemented a simple algo to parse some basic types on a best effort basis. This can be made more sophisticated over time.
This is done on the best effort basis and only for a small set of types because parsing default expressions from pg_catalog is tedious.
What kind of change does this PR introduce?
Feature
What is the current behavior?
UDFs with default arguments were not supported.
What is the new behavior?
UDFs with default arguments can be called now. Arguments with default values can be omitted in the GraphQL field's args.
Additional context
N/A