Re: [PATCH] pg_input_is_valid() reuses the first type name inside a PL/pgSQL loop

From: Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>
To: Xnerv Wang <xnervwang(at)gmail(dot)com>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org, John Naylor <johncnaylorls(at)gmail(dot)com>
Subject: Re: [PATCH] pg_input_is_valid() reuses the first type name inside a PL/pgSQL loop
Date: 2026-11-11 08:26:51
Message-ID: B3EE6745-4A0C-481A-AFC3-3A42ABDBDBDC@gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On Nov 11, 2026, at 16:04, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>
>
>
>> On Nov 10, 2026, at 16:11, Xnerv Wang <xnervwang(at)gmail(dot)com> wrote:
>>
>> Hi hackers,
>>
>> One of our tests checks exported values with pg_input_is_valid() in a
>> PL/pgSQL loop and it returned wrong results. I can reproduce the
>> problem on master and 19rc1:
>>
>> DO $$
>> DECLARE r record;
>> BEGIN
>> FOR r IN SELECT * FROM (VALUES ('42', 'integer'), ('hello', 'text'),
>> ('100000', 'smallint')) AS v(val, typ)
>> LOOP
>> RAISE NOTICE '% as %: % %', r.val, r.typ,
>> pg_input_is_valid(r.val, r.typ),
>> (pg_input_error_info(r.val, r.typ)).message;
>> END LOOP;
>> END $$;
>> NOTICE: 42 as integer: t <NULL>
>> NOTICE: hello as text: f invalid input syntax for type integer: "hello"
>> NOTICE: 100000 as smallint: t <NULL>
>>
>> The second and third calls should return t and f, but both validate
>> against integer, the type from the first call. pg_input_error_info()
>> has the same problem. The typmod is reused too: varchar(5) followed
>> by varchar(10) still checks against varchar(5).
>>
>> In misc.c, pg_input_is_valid_common() parses the type name only on the
>> first call when get_fn_expr_arg_stable() says the argument is stable.
>> That function returns true for a PARAM_EXTERN Param, which is how
>> PL/pgSQL passes variables. But a PL/pgSQL simple expression keeps its
>> ExprState, and so fn_extra, across evaluations within a transaction
>> while the variable changes. This logic came in with commit 1939d26282b,
>> in PostgreSQL 16.
>>
>> For a stable argument, the attached patch keeps the last type name
>> string it parsed and parses again when a later call passes a different
>> one. The new test in plpgsql_simple.sql (a similar loop plus varchar
>> typmod cases) fails without the fix, and check-world passes with it. A
>> constant type name now costs one extra name comparison per call: 5
>> million calls with 'int4' took 722 ms instead of 691 ms (-O2, no
>> assertions, median of 11 runs).
>>
>> Dropping typname_constant and always comparing names would be simpler
>> and would also fix this bug. A type name from a column would then be
>> parsed only when its text changes, rather than on every call as today,
>> so it would behave like a constant name does now. If the text stays
>> the same, a search_path change between two FETCHes from a cursor would
>> go unnoticed. I kept the flag so that non-constant names behave as
>> before, and can send the simpler version if others prefer it.
>>
>> Thanks,
>> Xnerv Wang
>> Amazon Web Services
>> <v1-0001-Fix-pg_input_is_valid-reusing-a-stale-type-in-PL-.patch>
>
> I can reproduce the issue with a slightly simpler example:
> ```
> evantest=# do $$
> evantest$# declare
> evantest$# typ text;
> evantest$# result boolean;
> evantest$# begin
> evantest$# for i in 1..2 loop
> evantest$# if i = 1 then
> evantest$# typ := 'integer';
> evantest$# else
> evantest$# typ := 'text';
> evantest$# end if;
> evantest$# result := pg_input_is_valid('abc', typ);
> evantest$# raise notice 'type=%, valid=%', typ, result;
> evantest$# end loop;
> evantest$# end $$;
> NOTICE: type=integer, valid=f
> NOTICE: type=text, valid=f
> ```
>
> After some debugging, I think the root cause is that get_fn_expr_arg_stable() returns true for both Const and PARAM_EXTERN. While an external parameter is stable within a single query execution, its value can change between evaluations of a cached PL/pgSQL expression.
>
> n pg_input_is_valid_common(), we have no info to distinguish whether the function is being called from PL/pgSQL. As you mentioned, the proposed fix introduces some overhead for genuine Const arguments, since their values cannot change.
>
> Since we have already added two fields to ValidIOData, I'm wondering if we could add another boolean field, say typname_stable, to store the result of get_fn_expr_arg_stable(), and use the existing typname_constant field to indicate whether the argument is a genuine Const. With this distinction, we could retain the original caching behavior for genuine constants, without comparing the type name on subsequent calls. We wouldn't even need to store the type name in ValidIOData for Const arguments, which would help mitigate the overhead.
>

I just had another thought. Comparing type names might not completely resolve the caching problem.

For example, if the type is a domain, a subsequent DDL could drop and recreate the domain with a different underlying type. When execution reaches the same pg_input_is_valid() call again, the type name is unchanged, but it would resolve to a different type OID, leaving the cache stale.

I don't think this is a practical use case, but it seems theoretically possible, assuming the cached FmgrInfo survives the DDL operation. I wouldn't want to block this patch over this concern. I'm just mentioning it for your consideration.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Daniel Gustafsson 2026-11-11 08:32:16 Re: [PATCH] RFC: new_oid_hook, to let an extension propose catalog OIDs
Previous Message Amit Langote 2026-11-11 08:23:20 Re: multi-row INSERT broken with a sublink in a target column subscript