Skip to content

test(analyser): check SQL function bodies in the Postgres regression harness - #939

Merged
psteinroe merged 1 commit into
mainfrom
test/regress-function-bodies
Oct 26, 2026
Merged

psteinroe merged 1 commit into
mainfrom
test/regress-function-bodies

Conversation

@psteinroe

@psteinroe psteinroe commented Oct 24, 2026 •

Copy link
Copy Markdown
Collaborator

Since #863, statements in SQL function bodies are only checked by the static typecheck rules, but the Postgres regression harness never analysed them, so a false positive in a body could slip through. This PR makes the harness (and the rule spec tests) analyse function bodies the way the workspace does.

Harness

For every CREATE FUNCTION ... LANGUAGE sql with a string body (AS '...' or AS $$...$$), the body is analysed right after the function, with the function's parameters in scope, by name and as $n. A finding in the body of a function Postgres accepted fails the test, like any other finding on an accepted statement. Findings in bodies are marked (body) in the per-version snapshots, and the header now counts the analysed bodies (347 on 15, 359 on 18).

To keep the harness and the workspace in sync, the extraction moved out of pgls_workspace:

  • pgls_query_ext::utils::sql_function_body finds the body text and its range (moved from get_sql_fn_body, unchanged).
  • pgls_catalog::resolve::FunctionContext::from_create_function builds the parameters the body sees. It replaces the workspace's get_sql_fn_signature → function_context conversion and the copy in the functionReturnTypeMismatch check. OUT / RETURNS TABLE columns are still left out.

The legacy SQLFunctionSignature is gone: TypecheckDiagnosticsMapper still returned it, but nothing has read it since #863.

False positives found

Across 15–18, one statement:

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

It is listed in KNOWN_FALSE_POSITIVES with its root cause, and shows up under "Known false positives" in the snapshots. The test fails if an entry stops matching, so fixes have to remove their entries. The fix is stacked on this PR.

The new body findings on statements Postgres rejected are all correct: select_into.sql (a table the function creates), window.sql (tenk1, which a fresh database doesn't have) and compression.sql (fipshash, from test_setup).

Not covered

Only bodies the workspace extracts today are analysed:

  • BEGIN ATOMIC and RETURN bodies (14–18 accepted functions per version) aren't extracted at all.
  • Bodies with more than one statement (3–5 per version) don't parse as a single statement, and the workspace reports them as a syntax error ("No root node found in parse result"). Fixed in fix(workspace): analyse every statement of a SQL function body #947.
  • '...' bodies with doubled quotes (4 per version) can't be found in the source text.

#918 and #941 (on the #888 stack) extract BEGIN ATOMIC, RETURN, '' and E'...' bodies. They rewrite the same get_sql_fn_body, which this PR moves to pgls_query_ext, so whichever lands second has to move their extraction behind the shared helper. The harness and the specs then pick up those bodies as well.

Tests: postgres_regress passes, the rule specs (rules_tests) also analyse bodies now and none of their snapshots changed, there's a new unit test for FunctionContext::from_create_function (moved from the workspace's signature tests), and a workspace test checks that a body statement gets its function's parameters.

@psteinroe
psteinroe merged commit 76e65b8 into main Oct 26, 2026
11 checks passed
@psteinroe
psteinroe deleted the test/regress-function-bodies branch October 26, 2026 07:13
psteinroe added a commit that referenced this pull request Oct 26, 2026
…unknown in the body (#942)

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:

```sql
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](https://github.com/postgres/postgres/blob/REL_18_6/src/backend/executor/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](https://github.com/postgres/postgres/blob/REL_18_6/src/backend/catalog/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.
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