From: Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>
To: Antonin Houska <ah@cybertec.at>
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 21:19:30 +0900
Message-ID: <5C66AE52.4020605@lab.ntt.co.jp> (raw)
In-Reply-To: <5C653888.70705@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>
<5C6251A7.40204@lab.ntt.co.jp>
<26618.1549971814@localhost>
<5C653888.70705@lab.ntt.co.jp>
(2019/02/14 18:44), Etsuro Fujita wrote:
> (2019/02/12 20:43), Antonin Houska wrote:
>> Etsuro Fujita<fujita.etsuro@lab.ntt.co.jp> wrote:
>>> Here are my review comments:
>>>
>>> root->upper_targets[UPPERREL_FINAL] = final_target;
>>> + root->upper_targets[UPPERREL_ORDERED] = final_target;
>>> root->upper_targets[UPPERREL_WINDOW] = sort_input_target;
>>> root->upper_targets[UPPERREL_GROUP_AGG] = grouping_target;
>>>
>>> I think that's a good idea. I think we'll soon need the PathTarget for
>>> UPPERREL_DISTINCT, so how about saving that as well like this?
>>>
>>> root->upper_targets[UPPERREL_FINAL] = final_target;
>>> + root->upper_targets[UPPERREL_ORDERED] = final_target;
>>> + root->upper_targets[UPPERREL_DISTINCT] = sort_input_target;
>>> root->upper_targets[UPPERREL_WINDOW] = sort_input_target;
>>> root->upper_targets[UPPERREL_GROUP_AGG] = grouping_target;
>>
>> I see nothing wrong about this.
>
> Cool!
>
>>> Another is about this:
>>>
>>> /*
>>> + * Set reltarget so that it's consistent with the paths. Also it's more
>>> + * convenient for FDW to find the target here.
>>> + */
>>> + ordered_rel->reltarget = target;
>>>
>>> I think this is correct if there are no SRFs in the targetlist, but
>>> otherwise
>>> I'm not sure this is really correct because the relation's reltarget
>>> would not
>>> match the target of paths added to the relation after
>>> adjust_paths_for_srfs().
>>
>> I wouldn't expect problem here. create_grouping_paths() also sets the
>> reltarget although adjust_paths_for_srfs() can be alled on grouped_rel
>> afterwards.
>
> Yeah, but as I said in a previous email, I think the root->upper_targets
> change would be enough at least currently for FDWs to consider
> performing post-UPPERREL_GROUP_AGG steps remotely, as shown in my
> patches, so I'm inclined to leave the retarget change for future work.
So, here is an updated patch. If there are no objections from you or
anyone else, I'll commit the patch as a preliminary one for what's
proposed in this thread.
Best regards,
Etsuro Fujita
Attachments:0001-Save-PathTargets-for-distinct-ordered-relations-in-r.patch (text/x-patch, 1.3K) download
From 5ade52b576cb8403d3a079fecf4c938225cc3bd7 Mon Sep 17 00:00:00 2001
From: Etsuro Fujita <efujita@postgresql.org>
Date: Fri, 15 Feb 2019 21:13:36 +0900
Subject: [PATCH] Save PathTargets for distinct/ordered relations in
root->upper_targets[]
Previously, we only saved the PathTargets for grouping, window and final
relations in root->upper_targets[] in grouping_planner. To improve the
convenience of extensions, save the PathTargets for distinct and ordered
relations as well.
Author: Antonin Houska, with an additional change by me
Discussion: https://postgr.es/m/10994.1549559088@localhost
---
src/backend/optimizer/plan/planner.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.cindex ddb86bd0c3..a4ec4456e5 100644--- a/src/backend/optimizer/plan/planner.c+++ b/src/backend/optimizer/plan/planner.c@@ -2035,6 +2035,8 @@ grouping_planner(PlannerInfo *root, bool inheritance_update,
* of the corresponding upperrels might not be needed for this query.
*/
root->upper_targets[UPPERREL_FINAL] = final_target;
+ root->upper_targets[UPPERREL_ORDERED] = final_target;+ root->upper_targets[UPPERREL_DISTINCT] = sort_input_target;
root->upper_targets[UPPERREL_WINDOW] = sort_input_target;
root->upper_targets[UPPERREL_GROUP_AGG] = grouping_target;
--
2.19.2
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, ah@cybertec.at, tgl@sss.pgh.pa.us, andrew@tao11.riddles.org.uk
Subject: Re: Problems with plan estimates in postgres_fdw
In-Reply-To: <5C66AE52.4020605@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