Re: BUG #18097: Immutable expression not allowed in generated at

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org
Cc: James Keener <jim(at)jimkeener(dot)com>
Subject: Re: BUG #18097: Immutable expression not allowed in generated at
Date: 2023-09-09 19:18:07
Message-ID: 2250243.1694287087@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

[ moving to pgsql-hackers ]

I wrote:
> Applying expression_planner() solves the problem because it inlines
> anytextcat(anynonarray,text), resolving that the required cast is
> numeric->text which is immutable. The code for generated expressions
> omits that step and arrives at the less desirable answer. I wonder
> where else we have the same issue.

After digging around, I could only find one other place where
outside-the-planner code was doing this wrong: AddRelationNewConstraints
can come to the wrong conclusion about whether it's safe to use
missingMode. So here's a patch series to resolve this. I split it
into three parts mostly because 0002 will only go back to v12 where
we added GENERATED, but the missingMode bug exists in v11.

There are a couple of points worth bikeshedding perhaps. I didn't
spend much thought on the wrapper functions' names, but it's surely
true that the semantic difference between contain_mutable_functions
and ContainMutableFunctions is quite un-apparent from those names.
Anybody got a better idea? It also seemed about fifty-fifty whether
to make the wrappers' argument types be Node * or Expr *. I stuck
with Expr * because that's what the predecessor code CheckMutability()
used, but that's not a very strong argument.

BTW, the test function in 0003 might look funny:

CREATE FUNCTION foolme(timestamptz DEFAULT clock_timestamp())
RETURNS timestamptz
IMMUTABLE AS 'select $1' LANGUAGE sql;

but AFAICS it's perfectly legit. The function itself is indeed immutable,
since it's only "select $1"; it's the default argument that's volatile.

I'll add this to the open CF 2023-11, but we really ought to
get it committed before that so we can ship these bug fixes in
November's releases.

regards, tom lane

Attachment Content-Type Size
v1-0001-Refactor-to-add-wrappers-for-contain_mutable-vola.patch text/x-diff 12.5 KB
v1-0002-Preprocess-column-GENERATED-expressions-before-ch.patch text/x-diff 3.1 KB
v1-0003-Preprocess-column-DEFAULT-expressions-before-chec.patch text/x-diff 3.5 KB

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Jeff Davis 2023-09-09 20:42:46 [16] ALTER SUBSCRIPTION ... SET (run_as_owner = ...) is a no-op
Previous Message Amit Kapila 2023-09-09 11:39:04 Re: BUG #18055: logical decoding core on AllocateSnapshotBuilder()

Browse pgsql-hackers by date

  From Date Subject
Next Message David Steele 2023-09-09 20:03:37 Re: Add const qualifiers
Previous Message Alvaro Herrera 2023-09-09 15:14:50 Re: Possibility to disable `ALTER SYSTEM`