Re: Why overhead of SPI is so large? - Mailing list pgsql-hackers

On 11.11.2019 20:22, Tom Lane wrote:
>
> None of those statements are true, in my experience.
>
> In general, this patch seems like it's learned nothing from our
> experiences with the late and unlamented exec_simple_check_node()
> (cf commit 00418c612).  Having a piece of plpgsql that has to know
> about all possible "simple expression" node types is just a major
> maintenance headache; but there is no short-cut solution that is
> really going to be acceptable.  Either you're unsafe, or you fail
> to optimize cases that users will complain about, or you write
> and maintain a lot of code.
>
> I'm also pretty displeased with the is-it-in-the-pg_catalog-schema
> tests.  Those are expensive, requiring additional catalog lookups,
> and they prove just about nothing.  There are extensions that shove
> stuff into pg_catalog (look no further than contrib/adminpack), or
> users could do it, so you really still are relying on the whole world
> to get immutability right.  If we're going to not trust immutability
> markings on user-defined objects, I'd be inclined to do it by
> checking that the object's OID is less than FirstGenbkiObjectId.
>
> But maybe we should just forget that bit of paranoia and rely solely
> on contain_mutable_functions().  That gets rid of the new maintenance
> requirement, and I think arguably it's OK.  An "immutable" function
> whose result depends on changes that were just made by the calling
> function is pretty obviously mislabeled, so people won't have much of
> a leg to stand on if they complain about that.  Pavel's argument
> upthread that people sometimes intentionally mislabel functions as
> immutable isn't really relevant: in his example, at least, the user
> is intentionally trying to get the function to be evaluated early.
> So whether it sees the effects of changes just made by the calling
> function doesn't seem like it would matter to such a user.
>
>             regards, tom lane

In my opinion contain_mutable_functions() is the best solution.
But if it is not acceptable, I will rewrite the patch in white-list fashion.

I do not understand the argument about expensive 
is-it-in-the-pg_catalog-schema test.
IsCatalogNameaspace is doing just simple comparison without any catalog 
lookups:

bool
IsCatalogNamespace(Oid namespaceId)
{
     return namespaceId == PG_CATALOG_NAMESPACE;
}

I may agree that it "proves nothing" if extension can put stuff in 
pg_catalog.
I can replace it with comparison with FirstGenbkiObjectId.
But all this makes sense only if using contain_mutable_function() is not 
acceptable.

-- 
Konstantin Knizhnik
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company