pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>
To: Pg Hackers <pgsql-hackers@postgresql.org>
Subject: postgres_fdw: estimate_path_cost_size fails to re-use cached costs
Date: Fri, 25 Jan 2019 20:33:07 +0900
Message-ID: <5C4AF3F3.4060409@lab.ntt.co.jp> (raw)

Hi,

I noticed yet another thing while updating the patch for pushing down
ORDER BY LIMIT.  Let me explain.  When costing foreign paths on the
basis of local statistics, we calculate/cache the costs of an unsorted
foreign path, and re-use them to estimate the costs of presorted foreign
paths, as shown below.  BUT: we fail to re-use them for some typical
queries, such as "select * from ft1 order by a", due to
fpinfo->rel_startup_cost=0, leading to doing the same cost calculation
repeatedly.

        /*
         * We will come here again and again with different set of pathkeys
         * that caller wants to cost. We don't need to calculate the cost of
         * bare scan each time. Instead, use the costs if we have cached
them
         * already.
         */
        if (fpinfo->rel_startup_cost > 0 && fpinfo->rel_total_cost > 0)
        {
            startup_cost = fpinfo->rel_startup_cost;
            run_cost = fpinfo->rel_total_cost - fpinfo->rel_startup_cost;
        }

I think we should use "fpinfo->rel_startup_cost >= 0" here, not
"fpinfo->rel_startup_cost > 0".  Also, it would be possible that the
total cost calculated is zero in corner cases (eg, seq_page_cost=0 and
cpu_tuple_cost=0 for the example), so I think we should change the total
cost part as well.  Attached is a patch for that.

Best regards,
Etsuro Fujita

Attachments:

  [text/x-patch] yet-another-fix-for-estimate_path_cost_size.patch (752B, ../5C4AF3F3.4060409@lab.ntt.co.jp/2-yet-another-fix-for-estimate_path_cost_size.patch)
  download | inline diff:
*** a/contrib/postgres_fdw/postgres_fdw.c
--- b/contrib/postgres_fdw/postgres_fdw.c
***************
*** 2625,2631 **** estimate_path_cost_size(PlannerInfo *root,
  		 * bare scan each time. Instead, use the costs if we have cached them
  		 * already.
  		 */
! 		if (fpinfo->rel_startup_cost > 0 && fpinfo->rel_total_cost > 0)
  		{
  			startup_cost = fpinfo->rel_startup_cost;
  			run_cost = fpinfo->rel_total_cost - fpinfo->rel_startup_cost;
--- 2625,2631 ----
  		 * bare scan each time. Instead, use the costs if we have cached them
  		 * already.
  		 */
! 		if (fpinfo->rel_startup_cost >= 0 && fpinfo->rel_total_cost >= 0)
  		{
  			startup_cost = fpinfo->rel_startup_cost;
  			run_cost = fpinfo->rel_total_cost - fpinfo->rel_startup_cost;

view thread (3+ messages)  latest in thread

Message-ID: <5C4AF3F3.4060409@lab.ntt.co.jp>
Permalink:  ../5C4AF3F3.4060409@lab.ntt.co.jp/
Also on:    postgresql.org/message-id/5C4AF3F3.4060409@lab.ntt.co.jp

 ·  · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: fujita.etsuro@lab.ntt.co.jp
  Subject: Re: postgres_fdw: estimate_path_cost_size fails to re-use cached costs
  In-Reply-To: <5C4AF3F3.4060409@lab.ntt.co.jp>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox