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
