postgres.git / summary / log / commit / refs

commit    6f0f98bb0bced134c09b7acf8fe35ff3a6f1bbd2
Author:   Tom Lane <tgl@sss.pgh.pa.us>
Date:     Mon Apr 17 19:29:00 2017 +0000

    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


 src/backend/optimizer/plan/createplan.c | 14 +++++++++++---
 1 file 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;
 
@@ -516,6 +517,13 @@ use_physical_tlist(PlannerInfo *root, RelOptInfo *rel)
 	if (rel->reloptkind != RELOPT_BASEREL)
 		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


[parent: b6e6ae1dc6a6]