On Wed, Nov 11, 2026 at 12:26 AM Chao Li <[email protected]> wrote:
>> 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 did that in the attached v2: I check whether the second FuncExpr
argument is a Const, since get_fn_expr_arg_stable() returns true for
both Const and external Param. A Const type name is parsed on the first
call and reused, as on master, without storing or comparing the name.
An external Param, as used for PL/pgSQL variables, retains the last
parsed name and is parsed again when it changes.

I timed 5 million calls with the constant 'int4': 704 ms with v2,
717 ms on unpatched master (-O2, no assertions, median of 11 runs).
The test is the same as in v1; it fails without the fix, and
check-world passes with v2.

> 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 ran this on unpatched master:

  CREATE DOMAIN dd AS int;
  DO $$
  BEGIN
    FOR i IN 1..2 LOOP
      IF i = 2 THEN DROP DOMAIN dd; CREATE DOMAIN dd AS text; END IF;
      RAISE NOTICE 'const name, iteration %: %', i,
pg_input_is_valid('abc', 'dd');
    END LOOP;
  END $$;
  NOTICE:  const name, iteration 1: f
  NOTICE:  const name, iteration 2: f

I get the same output with v2. With the type name in a PL/pgSQL
variable instead of the constant 'dd', I also get f on the second
iteration with v2; it should return t because 'abc' is valid text.

Thanks,
Xnerv Wang
Amazon Web Services

On Wed, Nov 11, 2026 at 12:27 AM Chao Li <[email protected]> wrote:

>
>
> > On Nov 11, 2026, at 16:04, Chao Li <[email protected]> wrote:
> >
> >
> >
> >> On Nov 10, 2026, at 16:11, Xnerv Wang <[email protected]> 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/
>
>
>
>
>

Attachment: v2-0001-Fix-pg_input_is_valid-reusing-a-stale-type-in-PL-.patch
Description: Binary data

Reply via email to