diff options
| author | Tom Lane <tgl@sss.pgh.pa.us> | 2017-04-17 15:29:00 -0400 |
|---|---|---|
| committer | Tom Lane <tgl@sss.pgh.pa.us> | 2017-04-17 15:29:00 -0400 |
| commit | 6f0f98bb0bced134c09b7acf8fe35ff3a6f1bbd2 (patch) | |
| tree | a61a2a9da76050a9a134f8a492b152d24d5fd6b3 /src | |
| parent | b6e6ae1dc6a647ae6c846c79a8515802a29ebaba (diff) | |
| download | postgresql-6f0f98bb0bced134c09b7acf8fe35ff3a6f1bbd2.tar.gz postgresql-6f0f98bb0bced134c09b7acf8fe35ff3a6f1bbd2.zip | |
Always build a custom plan node's targetlist from the path's pathtarget.
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
Diffstat (limited to 'src')
| -rw-r--r-- | src/backend/optimizer/plan/createplan.c | 14 |
1 files changed, 11 insertions, 3 deletions
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index 6af68b08d27..c464d054c94 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -47,7 +47,7 @@ static Plan *create_plan_recurse(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 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 Plan *create_gating_plan(PlannerInfo *root, Plan *plan, List *quals); 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 * occur.) */ - if (use_physical_tlist(root, rel)) + if (use_physical_tlist(root, best_path)) { if (best_path->pathtype == T_IndexOnlyScan) { @@ -493,8 +493,9 @@ build_path_tlist(PlannerInfo *root, Path *path) * rather than only those Vars actually referenced. */ static bool -use_physical_tlist(PlannerInfo *root, RelOptInfo *rel) +use_physical_tlist(PlannerInfo *root, Path *path) { + RelOptInfo *rel = path->parent; int i; ListCell *lc; @@ -517,6 +518,13 @@ use_physical_tlist(PlannerInfo *root, RelOptInfo *rel) 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. * (This could possibly be fixed but would take some fragile assumptions * in setrefs.c, I think.) |