pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Antonin Houska <ah@cybertec.at>
To: Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Andrew Gierth <andrew@tao11.riddles.org.uk>
Cc: pgsql-hackers@postgresql.org
Subject: Re: Problems with plan estimates in postgres_fdw
Date: Fri, 15 Feb 2019 13:46:02 +0100
Message-ID: <16616.1550234762@localhost> (raw)
In-Reply-To: <5C669DFD.30002@lab.ntt.co.jp>
References: <87pnz1aby9.fsf@news-spur.riddles.org.uk>
	<1175.1533220908@sss.pgh.pa.us>
	<5BB739CD.6040002@lab.ntt.co.jp>
	<5BBC4140.50403@lab.ntt.co.jp>
	<5C17A00E.5000005@lab.ntt.co.jp>
	<5C232F39.9060509@lab.ntt.co.jp>
	<5C25C7B5.2010802@lab.ntt.co.jp>
	<5C518631.5080401@lab.ntt.co.jp>
	<10994.1549559088@localhost>
	<5C5D778F.3080501@lab.ntt.co.jp>
	<19433.1549962210@localhost>
	<5C669DFD.30002@lab.ntt.co.jp>

Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp> wrote:

> (2019/02/12 18:03), Antonin Houska wrote:
> > Etsuro Fujita<fujita.etsuro@lab.ntt.co.jp>  wrote:
> >> (2019/02/08 2:04), Antonin Houska wrote:
> >>>
> >>> And maybe related problem: why should FDW care about the effect of
> >>> apply_scanjoin_target_to_paths() like you claim to do here?
> >>>
> >>> 	/*
> >>> 	 * If this is UPPERREL_ORDERED and/or UPPERREL_FINAL steps on the
> >>> 	 * final scan/join relation, the costs obtained from the cache
> >>> 	 * wouldn't yet contain the eval cost for the final scan/join target,
> >>> 	 * which would have been updated by apply_scanjoin_target_to_paths();
> >>> 	 * add the eval cost now.
> >>> 	 */
> >>> 	if (fpextra&&   !IS_UPPER_REL(foreignrel))
> >>> 	{
> >>> 		/* The costs should have been obtained from the cache. */
> >>> 		Assert(fpinfo->rel_startup_cost>= 0&&
> >>> 			fpinfo->rel_total_cost>= 0);
> >>>
> >>> 		startup_cost += foreignrel->reltarget->cost.startup;
> >>> 		run_cost += foreignrel->reltarget->cost.per_tuple * rows;
> >>> 	}
> >>>
> >>> I think it should not, whether "foreignrel" means "input_rel" or "output_rel"
> >>> from the perspective of postgresGetForeignUpperPaths().
> >>
> >> I added this before costing the sort operation below, because 1) the cost of
> >> evaluating the scan/join target might not be zero (consider the case where
> >> sort columns are not simple Vars, for example) and 2) the cost of sorting
> >> takes into account the underlying path's startup/total costs.  Maybe I'm
> >> missing something, though.
> >
> > My understanding is that the "input_rel" argument of
> > postgresGetForeignUpperPaths() should already have been processed by
> > postgresGetForeignPaths() or postgresGetForeignJoinPaths() (or actually by
> > postgresGetForeignUpperPaths() called with different "stage" too). Therefore
> > it should already have the "fdw_private" field initialized. The estimates in
> > the corresponding PgFdwRelationInfo should already include the reltarget
> > costs, as set previously by estimate_path_cost_size().
> 
> Right, but what I'm saying here is: the costs of evaluating the final
> scan/join target updated by apply_scanjoin_target_to_paths() wouldn't be yet
> included in the costs we calculated using local statistics when called from
> postgresGetForeignPaths() or postgresGetForeignJoinPaths(). Let me explain
> using an example:
> 
>     SELECT a+b FROM foreign_table LIMIT 10;
> 
> For this query, the reltarget for the foreign_table would be {a, b} when
> called from postgresGetForeignPaths() (see build_base_rel_tlists()). The costs
> of evaluating simple Vars are zeroes, so we wouldn't charge any costs for
> tlist evaluation when estimating the costs of a basic foreign table scan in
> that callback routine, but the final scan target, with which the reltarget
> will be replaced later by apply_scanjoin_target_to_paths(), would be {a+b}, so
> we need to adjust the costs of the basic foreign table scan, cached in the
> PgFdwRelationInfo for the input_rel (ie, the foreign_table), so that eval
> costs for the replaced tlist are included when called from
> postgresGetForeignUpperPaths() with the UPPERREL_FINAL stage, as the costs of
> evaluating the expression 'a+b' wouldn't be zeroes.

ok, I understand now. I assume that the patch

https://www.postgresql.org/message-id/5C66A056.60007%40lab.ntt.co.jp

obsoletes the code snippet we discussed above.

-- 
Antonin Houska
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26, A-2700 Wiener Neustadt
Web: https://www.cybertec-postgresql.com




view thread (48+ messages)  latest in thread

Message-ID: <16616.1550234762@localhost>
Permalink:  ../16616.1550234762@localhost/
Also on:    postgresql.org/message-id/16616.1550234762@localhost

 · 

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: ah@cybertec.at, fujita.etsuro@lab.ntt.co.jp, tgl@sss.pgh.pa.us, andrew@tao11.riddles.org.uk
  Subject: Re: Problems with plan estimates in postgres_fdw
  In-Reply-To: <16616.1550234762@localhost>

* 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