Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 6c73b39

Browse files
committed
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
1 parent 1ec36a9 commit 6c73b39

File tree

1 file changed

+9
-0
lines changed

1 file changed

+9
-0
lines changed

src/backend/optimizer/plan/createplan.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -761,6 +761,15 @@ use_physical_tlist(PlannerInfo *root, Path *path, int flags)
761761
if (rel->reloptkind != RELOPT_BASEREL)
762762
return false;
763763

764+
/*
765+
* Also, don't do it to a CustomPath; the premise that we're extracting
766+
* columns from a simple physical tuple is unlikely to hold for those.
767+
* (When it does make sense, the custom path creator can set up the path's
768+
* pathtarget that way.)
769+
*/
770+
if (IsA(path, CustomPath))
771+
return false;
772+
764773
/*
765774
* Can't do it if any system columns or whole-row Vars are requested.
766775
* (This could possibly be fixed but would take some fragile assumptions

0 commit comments

Comments
 (0)