database/postgres
database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-06-05 23:56:58 +03:00
Code

Always build a custom plan node's targetlist from the path's pathtarget.

Browse Source 
We were applying the use_physical_tlist optimization to all relation
scan plans, even those implemented by custom scan providers.  However,
that's a bad idea for a couple of reasons.  The custom provider might
be unable to provide columns that it hadn't expected to be asked for
(for example, the custom scan might depend on an index-only scan).
Even more to the point, there's no good reason to suppose that this
"optimization" is a win for a custom scan; whatever the custom provider
is doing is likely not based on simply returning physical heap tuples.
(As a counterexample, if the custom scan is an interface to a column store,
demanding all columns would be a huge loss.)  If it is a win, the custom
provider could make that decision for itself and insert a suitable
pathtarget into the path, anyway.

Per discussion with Dmitry Ivanov.  Back-patch to 9.5 where custom scan
support was introduced.  The argument that the custom provider can adjust
the behavior by changing the pathtarget only applies to 9.6+, but on
balance it seems more likely that use_physical_tlist will hurt custom
scans than help them.

Discussion: https://postgr.es/m/e29ddd30-8ef9-4da5-a50b-2bb7b8c7198d@postgrespro.ru
This commit is contained in:
Tom Lane 2017-04-17 15:29:00 -04:00
parent b6e6ae1dc6
commit 6f0f98bb0b
1 changed files with 11 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
src/backend/optimizer/plan/createplan.c
View File

@ -47,7 +47,7 @@
static Plan *create_plan_recurse(PlannerInfo *root, Path *best_path); static Plan *create_plan_recurse(PlannerInfo *root, Path *best_path);
static Plan *create_scan_plan(PlannerInfo *root, Path *best_path); static Plan *create_scan_plan(PlannerInfo *root, Path *best_path);
static List *build_path_tlist(PlannerInfo *root, Path *path); static List *build_path_tlist(PlannerInfo *root, Path *path);
static bool use_physical_tlist(PlannerInfo *root, RelOptInfo *rel); static bool use_physical_tlist(PlannerInfo *root, Path *path);
static void disuse_physical_tlist(PlannerInfo *root, Plan *plan, Path *path); static void disuse_physical_tlist(PlannerInfo *root, Plan *plan, Path *path);
static Plan *create_gating_plan(PlannerInfo *root, Plan *plan, List *quals); static Plan *create_gating_plan(PlannerInfo *root, Plan *plan, List *quals);
static Plan *create_join_plan(PlannerInfo *root, JoinPath *best_path); static Plan *create_join_plan(PlannerInfo *root, JoinPath *best_path);
@ -303,7 +303,7 @@ create_scan_plan(PlannerInfo *root, Path *best_path)
* planner.c may replace the tlist we generate here, forcing projection to * planner.c may replace the tlist we generate here, forcing projection to
* occur.) * occur.)
*/ */
if (use_physical_tlist(root, rel)) if (use_physical_tlist(root, best_path))
{ {
if (best_path->pathtype == T_IndexOnlyScan) if (best_path->pathtype == T_IndexOnlyScan)
{ {
@ -493,8 +493,9 @@ build_path_tlist(PlannerInfo *root, Path *path)
* rather than only those Vars actually referenced. * rather than only those Vars actually referenced.
*/ */
static bool static bool
use_physical_tlist(PlannerInfo *root, RelOptInfo *rel) use_physical_tlist(PlannerInfo *root, Path *path)
{ {
RelOptInfo *rel = path->parent;
int i; int i;
ListCell *lc; ListCell *lc;
@ -516,6 +517,13 @@ use_physical_tlist(PlannerInfo *root, RelOptInfo *rel)
if (rel->reloptkind != RELOPT_BASEREL) if (rel->reloptkind != RELOPT_BASEREL)
return false; return false;
/*
* Also, don't do it to a CustomPath; the premise that we're extracting
* columns from a simple physical tuple is unlikely to hold for those.
*/
if (IsA(path, CustomPath))
return false;
/* /*
* Can't do it if any system columns or whole-row Vars are requested. * Can't do it if any system columns or whole-row Vars are requested.
* (This could possibly be fixed but would take some fragile assumptions * (This could possibly be fixed but would take some fragile assumptions