Re: Why overhead of SPI is so large?

From: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>
To: k(dot)knizhnik(at)postgrespro(dot)ru
Cc: tgl(at)sss(dot)pgh(dot)pa(dot)us, pavel(dot)stehule(at)gmail(dot)com, tsunakawa(dot)takay(at)jp(dot)fujitsu(dot)com, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Why overhead of SPI is so large?
Date: 2019-11-13 01:25:47
Message-ID: 20191113.102547.2117555293382778346.horikyota.ntt@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

At Tue, 12 Nov 2019 11:27:24 +0300, Konstantin Knizhnik <k(dot)knizhnik(at)postgrespro(dot)ru> wrote in
>
>
> 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 agree for just relying on contain_mutable_functions for the same
reasons to Tom.

> 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.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kyotaro Horiguchi 2019-11-13 01:43:35 Re: [PATCH][BUG_FIX] Potential null pointer dereferencing.
Previous Message Masahiko Sawada 2019-11-13 01:22:56 Re: [HACKERS] Block level parallel vacuum