From: | Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com> |
---|---|
To: | pavel(dot)stehule(at)gmail(dot)com |
Cc: | k(dot)knizhnik(at)postgrespro(dot)ru, 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-07 12:03:17 |
Message-ID: | 20191107.210317.2207671169828824912.horikyota.ntt@gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hello.
At Tue, 5 Nov 2019 22:14:40 +0100, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote in
> Hi
>
> pá 23. 8. 2019 v 16:32 odesílatel Konstantin Knizhnik <
> k(dot)knizhnik(at)postgrespro(dot)ru> napsal:
>
> >
> >
> > On 23.08.2019 14:42, Pavel Stehule wrote:
> >
> >
> > In reality it is not IMMUTABLE function. On second hand, there are lot of
> > application that depends on this behave.
> >
> > It is well know trick how to reduce estimation errors related to JOINs.
> > When immutable function has constant parameters, then it is evaluated in
> > planning time.
> >
> > So sometimes was necessary to use
> >
> > SELECT ... FROM tab WHERE foreign_key = immutable_function('constant
> > parameter')
> >
> > instead JOIN.
> >
> > It is ugly, but it is working perfectly. I think so until we will have
> > multi table statistics, this behave should be available in Postgres.
> >
> > Sure, this function should not be used for functional indexes.
> >
> >
> >
> > What about the following version of the patch?
> >
>
> I am sending review of this small patch.
>
> This small patch reduce a overhead of usage buildin immutable functions in
> volatile functions with simple trick. Starts snapshot only when it is
> necessary.
>
> In decrease runtime time about 25 % on this small example.
>
> do $$
> declare i int;
> begin
> i := 0;
> while i < 10000000
> loop
> i := i + 1;
> end loop;
> end;
> $$;
>
> If there are more expressions, then speedup can be more interesting. If
> there are other bottlenecks, then the speedup will be less. 25% is not bad,
> so we want to this feature.
>
> I believe so similar method can be used more aggressively with more
> significant performance benefit, but this is low hanging fruit and isn't
> reason to wait for future.
>
> This patch doesn't introduce any new feature, so new tests and new doc is
> not necessary.
> The patch is readable, well formatted, only comments are too long. I fixed
> it.
> All tests passed
> I fixed few warnings, and I reformated little bit function
> expr_needs_snapshot to use if instead case, what is more usual in these
> cases.
>
> I think so this code can be marked as ready for commit
I have some comments on this.
expr_needs_snapshot checks out some of the node already checked out in
exec_simple_check_plan but not all. However I don't see the criteria
of the choice.
I might be too worrying, but maybe we should write the function in
white-listed way, that is, expr_needs_snapshot returns false only if
the whole tree consists of only the node types known to the
function. And any unknown node makes the function return true
immediately.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
From | Date | Subject | |
---|---|---|---|
Next Message | Pavel Stehule | 2019-11-07 12:09:52 | Re: Why overhead of SPI is so large? |
Previous Message | 曾文旌 (义从) | 2019-11-07 11:49:29 | Re: [Proposal] Global temporary tables |