public inbox for [email protected]  

help / color / mirror / Atom feed

From: Tom Lane <[email protected]>
To: [email protected]
Cc: James Keener <[email protected]>
Subject: Re: BUG #18097: Immutable expression not allowed in generated at
Date: Sat, 09 Sep 2023 18:56:42 -0400
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<CAKFQuwZW=wyr_Nfi8Ww+U-jxdh4tvztvMjT23iv2Ly+Oc0n5eQ@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<[email protected]>

I wrote:
> 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.

Argh ... I forgot to mention that there's one other place that
this patch series doesn't address, which is that publicationcmds.c's
check_simple_rowfilter_expr() also checks for volatile functions
without having preprocessed the expression.  I'm not entirely sure
that there's a reachable problem in the direction of underestimating
the expression's volatility, given that that logic rejects non-builtin
functions entirely: it seems unlikely that any immutable builtin
function would have a volatile default expression.  But it definitely
seems possible that there would be a problem in the other direction,
leading to rejecting row filter expressions that we'd like to allow,
much as in bug #18097.

I'm not sure about a good way to resolve this.  Simply applying
expression simplification ahead of the check would break the code's
intent of rejecting non-builtin operators, in the case where such
an operator resolves to an inline-able builtin function.  I find
the entire design of check_simple_rowfilter_expr pretty questionable
anyway, and there are a bunch of dubious (and undocumented) individual
decisions like allowing whole-row Vars, using FirstNormalObjectId
rather than FirstUnpinnedObjectId as the cutoff, etc.  So I'm not
planning to touch that code, but somebody who was paying attention
when it was written might want to take a second look.

			regards, tom lane

view thread (8+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: BUG #18097: Immutable expression not allowed in generated at
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox