Skip to content

PG17: Serialize retvarno and out_param_varno in PL/pgSQL parse output - #356

Open
pyramation wants to merge 2 commits into
pganalyze:17-latestfrom
constructive-io:fix/plpgsql-retvarno-17
Open

PG17: Serialize retvarno and out_param_varno in PL/pgSQL parse output#356
pyramation wants to merge 2 commits into
pganalyze:17-latestfrom
constructive-io:fix/plpgsql-retvarno-17

Conversation

@pyramation

Copy link
Copy Markdown
Contributor

I built and tested this against the full 23,000 statements in pgsql-parser, and a slew of other tests...

The JSON output of pg_query_parse_plpgsql omits two fields needed to faithfully reconstruct RETURN / RETURN NEXT statements:

  • PLpgSQL_stmt_return.retvarno / PLpgSQL_stmt_return_next.retvarno — without it, RETURN NEXT <variable> is indistinguishable from a bare RETURN NEXT;, so any consumer round-tripping the AST back to source drops the returned variable.
  • PLpgSQL_function.out_param_varno — needed to tell the two apart in OUT-parameter functions, where the compiler sets retvarno implicitly even for a bare RETURN NEXT; (and where emitting the variable name back is invalid syntax).

This adds both fields to the serialized output. Existing output is unchanged except for the added keys.

…JSON output

Backport of the PG 18 fix (pganalyze#355 fix 3) to 17: retvarno was
commented out in dump_return/dump_return_next, so variable returns like
RETURN NEXT r lost the returned variable entirely. Emit it only when
>= 0 (-1 means the return uses expr instead of a datum). Golden
plpgsql_samples.expected.json regenerated.
@lfittl

lfittl commented Jul 21, 2026

Copy link
Copy Markdown
Member

@pyramation Thanks for the contribution!

Can you remind me why you opened this against 17? Is that because of the current challenges with PL/pgSQL parsing on the 18 branch, and this way you're unblocked on having this working?

@pyramation

Copy link
Copy Markdown
Contributor Author

I was doing it to keep things in sync, but we can close this and focus on 18 from here on out. I'm fine with 18 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants