Skip to content

fix(catalog): leave the types of polymorphic SQL function parameters unknown in the body - #942

Merged
psteinroe merged 4 commits into
mainfrom
fix/polymorphic-function-params
Oct 26, 2026
Merged

psteinroe merged 4 commits into
mainfrom
fix/polymorphic-function-params

Conversation

@psteinroe

Copy link
Copy Markdown
Collaborator

Stacked on #939.

The typecheck rules gave the parameters of a SQL function their declared types inside the body, including polymorphic ones. With $1 anyelement, a call like array_prepend($1, $2) was matched against array_prepend(anycompatible, anycompatiblearray) with anyelement as the actual argument type, which no candidate accepts:

create function formarray(anyelement, variadic anyarray) returns anyarray as $$
  select array_prepend($1, $2);
$$ language sql immutable strict;
-- before: functionArgumentMismatch: Function array_prepend(anyelement, anyarray) does not exist.
-- after: nothing

This is the false positive #939 found in polymorphism.sql.

Postgres never analyses such a body with the declared types. When check_function_bodies is on, fmgr_sql_validator only raw-parses the body of a function with polymorphic arguments. At call time, prepare_sql_fn_parse_info swaps each polymorphic parameter type for the type of the call's argument before it parses the body (functions.c#L286). So the type of a polymorphic parameter in the body is whatever the caller passes, and we can't know it from the body. BEGIN ATOMIC bodies, which are analysed at creation with the declared types, aren't affected: Postgres rejects polymorphic arguments there ("SQL function with unquoted function body cannot have polymorphic arguments").

The fix: function_param_type now returns unknown for parameters of a pseudo-type. Polymorphic types are the only pseudo-types SQL functions accept as parameters (pg_proc.c#L873). Expressions that use these parameters are therefore unknown, and nothing is reported for them. Parameters of concrete types (including arrays) still have their declared types.

Tests: a new functionArgumentMismatch/function_parameters.sql spec. It checks that a concrete parameter is still checked (length(n) with n int is reported) and that array_prepend($1, $2) and length(a) with polymorphic parameters aren't. The polymorphism entry is gone from the harness's KNOWN_FALSE_POSITIVES, which is now empty. The regression snapshots only lose their "Known false positives" section.

Not covered: we don't infer the result types of polymorphic SQL functions at call sites from the body. That already went through the function's declared signature and is unchanged.

@psteinroe
psteinroe changed the base branch from test/regress-function-bodies to main October 26, 2026 07:13
@psteinroe
psteinroe merged commit 9de9662 into main Oct 26, 2026
11 checks passed
@psteinroe
psteinroe deleted the fix/polymorphic-function-params branch October 26, 2026 08:20
psteinroe added a commit that referenced this pull request Oct 26, 2026
Stacked on #942.

The workspace treated the body of a `LANGUAGE sql` function as a single statement. A body with several statements doesn't parse as one, so it was reported as a syntax error, and none of its statements were linted or typechecked. An empty body got the same error:

```sql
create function add_user(p_name text) returns int language sql as $$
  insert into users (name) values (p_name);
  select count(*)::int from users;
$$;
-- before: syntax error "No root node found in parse result" over the whole body
-- after: no syntax error; both statements are analysed with `p_name` in scope

create function noop() returns void language sql as $$ $$;
-- before: the same syntax error
-- after: nothing
```

`SqlFunctionBody::statements` (in `pgls_query_ext`, next to `sql_function_body`) splits the body the way Postgres does (`pg_parse_query`), using the statement locations libpg_query returns. Each statement becomes a child of the `CREATE FUNCTION`, with its range in the document and its trailing semicolon, like top-level statements. Two cases stay as before:

- A body with a single statement is still one child spanning the whole body, so hover, completions and the formatter see exactly what they saw before.
- A body that doesn't parse is also still one child, so its syntax error is still reported for the body.

An empty body now has no children.

The formatter only rewrote bodies with exactly one child, and it now checks that explicitly. A body with several statements is left as written. Before this PR it was skipped because it didn't parse; without the check, the last statement would have replaced the whole body.

The regression harness and the rule specs use the same split, so they now analyse 357–373 body statements per version, up from 347–359. There are no new findings on accepted statements and no changes to rejected-statement findings, only the header counts.

Tests: unit tests for `statements()` (single statement, several with and without semicolons, empty, unparsable), document tests for a body with several statements (no syntax error, function context, document ranges) and an empty body, and a formatter test that keeps a body with several statements as written.

Not covered: `BEGIN ATOMIC`, `RETURN`, `''` and `E'...'` bodies. #918 and #941 (on the #888 stack) extract those, and they conflict with this stack in the body extraction (`document.rs`, `sql_function.rs`). Whichever lands second should split string bodies with `SqlFunctionBody::statements`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant