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-12 01:37:28
Message-ID: B6A2F813-6ABF-4973-B4EF-0131D6021FAB@gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On Nov 12, 2026, at 03:58, Xnerv Wang <xnervwang(at)gmail(dot)com> wrote:
>
> On Wed, Nov 11, 2026 at 12:26 AM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> 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.
>

So, you've confirmed the problem. It's not limited to domains. Dropping and recreating a user-defined type under the same name can cause the same issue. Even a change to search_path could cause the same type name to resolve to a different type OID.

I'm afraid we don't have a reliable way to determine if the resolved type has changed in pg_input_is_valid_common() without either resolving the type OID. This means the patch only partially addresses the caching problem. Given that, I'd wait and see if the committers have any concerns about this partial fix.

It feels to me that, without some refactoring, the current caching mechanism cannot reliably handle all these cases.

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 Ishjaap Singh 2026-11-12 02:09:09 Re: [PATCH] psql: Fix REPACK index completion after a column list
Previous Message David Rowley 2026-11-12 00:46:39 Re: Memoize: stale entry pointer after cache_store_tuple()