Re: Why overhead of SPI is so large?

From: Konstantin Knizhnik <k(dot)knizhnik(at)postgrespro(dot)ru>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, "Tsunakawa, Takayuki" <tsunakawa(dot)takay(at)jp(dot)fujitsu(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Why overhead of SPI is so large?
Date: 2019-11-12 08:27:24
Message-ID: 780a2166-0007-b7cc-2945-db59dba8ba09@postgrespro.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message PG Bug reporting form 2019-11-12 08:29:10 BUG #16108: Colorization to the output of command-line has unproperly behaviors at Windows platform
Previous Message ideriha.takeshi@fujitsu.com 2019-11-12 07:50:44 RE: Built-in connection pooler