Dilip Kumar <dilipbalaut@gmail.com> writes:
> On Thu, Jul 14, 2016 at 1:37 PM, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp
>> wrote:
>> Can we say that pg_get_indexdef() has "side-effects" because it can error
>> like this? Shouldn't such a function be marked *volatile*? Because if I
>> do so by updating pg_proc, the plan changes (perhaps) to a safe one in
>> this context:
> That is another option, but by nature this function is not actually
> volatile, because if clause is on *pg_index* indexrelid then it can be
> pushed down.
> So I think changing the view definition and calling this function on
> indexrelid will remove the error. So I think
> correct fix is to change view definition, as I proposed in above patch.
I'm unimpressed with that solution. It cannot be back-patched, because
it forces an initdb; and while it might avoid the issue for this specific
use of this specific view, there are plenty of other scenarios in which
someone could apply pg_get_indexdef() and get a similar error. For
example, it's not at all uncommon to see reports of failures like this
for a just-dropped index (when the query's snapshot can still see the
index's catalog entries, but internally the backend realizes it's gone).
One recent example is
https://www.postgresql.org/message-id/flat/CAHnozTjkOP8o0MqNZtuc5HgPD1tLRmTQvZAKY%2BRNNvOkmMbK0A%40mail.gmail.com
We've dealt with similar issues in places like pg_relation_size() by
making the functions return NULL instead of throwing an error for an
unmatched argument OID. It's pretty tempting to make the ruleutils.c
functions behave similarly. We'd have to look at the usages in pg_dump
and psql to see if any of them need adjustment; I imagine pg_dump at least
would need to be taught to fail if it gets back a NULL for the index
definition.
regards, tom lane