Hi, Robert, I CCed you because you are the author of that commit. Before that commit ("Rewrite the code that applies scan/join targets to paths."), apply_scanjoin_target_to_paths() had a boolean parameter named modify_in_place, and used apply_projection_to_path(), not create_projection_path(), to adjust scan/join paths when modify_in_place was true, which allowed us to save cycles at plan creation time by avoiding creating projection paths, which I think would be a good thing, but that commit removed that. Why?
The real reason for this question is: I noticed that projection paths put on foreign paths will make it hard for FDWs to detect whether there is an already-well-enough-sorted remote path in the pathlist for the final scan/join relation as an input relation to GetForeignUpperPaths() for the UPPERREL_ORDERED step (the IsA(path, ForeignPath) test would not work well enough to detect remote paths!), so I'm wondering whether we could revive that parameter like the attached, to avoid the overhead at plan creation time and to make the FDW work easy. Maybe I'm missing something, though. Best regards, Etsuro Fujita
*** a/contrib/postgres_fdw/expected/postgres_fdw.out --- b/contrib/postgres_fdw/expected/postgres_fdw.out *************** *** 2815,2821 **** select sum(c1) from ft1 group by c2 having avg(c1 * (random() <= 1)::int) > 100 Group Key: ft1.c2 Filter: (avg((ft1.c1 * ((random() <= '1'::double precision))::integer)) > '100'::numeric) -> Foreign Scan on public.ft1 ! Output: c1, c2 Remote SQL: SELECT "C 1", c2 FROM "S 1"."T 1" (10 rows) --- 2815,2821 ---- Group Key: ft1.c2 Filter: (avg((ft1.c1 * ((random() <= '1'::double precision))::integer)) > '100'::numeric) -> Foreign Scan on public.ft1 ! Output: c2, c1 Remote SQL: SELECT "C 1", c2 FROM "S 1"."T 1" (10 rows) *************** *** 3039,3045 **** select sum(c1) filter (where (c1 / c1) * random() <= 1) from ft1 group by c2 ord Output: sum(c1) FILTER (WHERE ((((c1 / c1))::double precision * random()) <= '1'::double precision)), c2 Group Key: ft1.c2 -> Foreign Scan on public.ft1 ! Output: c1, c2 Remote SQL: SELECT "C 1", c2 FROM "S 1"."T 1" (9 rows) --- 3039,3045 ---- Output: sum(c1) FILTER (WHERE ((((c1 / c1))::double precision * random()) <= '1'::double precision)), c2 Group Key: ft1.c2 -> Foreign Scan on public.ft1 ! Output: c2, c1 Remote SQL: SELECT "C 1", c2 FROM "S 1"."T 1" (9 rows) *************** *** 3213,3219 **** select array_agg(c1 order by c1 using operator(public.<^)) from ft2 where c2 = 6 Output: array_agg(c1 ORDER BY c1 USING <^ NULLS LAST), c2 Group Key: ft2.c2 -> Foreign Scan on public.ft2 ! Output: c1, c2 Remote SQL: SELECT "C 1", c2 FROM "S 1"."T 1" WHERE (("C 1" < 100)) AND ((c2 = 6)) (6 rows) --- 3213,3219 ---- Output: array_agg(c1 ORDER BY c1 USING <^ NULLS LAST), c2 Group Key: ft2.c2 -> Foreign Scan on public.ft2 ! Output: c2, c1 Remote SQL: SELECT "C 1", c2 FROM "S 1"."T 1" WHERE (("C 1" < 100)) AND ((c2 = 6)) (6 rows) *************** *** 3261,3267 **** select array_agg(c1 order by c1 using operator(public.<^)) from ft2 where c2 = 6 Output: array_agg(c1 ORDER BY c1 USING <^ NULLS LAST), c2 Group Key: ft2.c2 -> Foreign Scan on public.ft2 ! Output: c1, c2 Remote SQL: SELECT "C 1", c2 FROM "S 1"."T 1" WHERE (("C 1" < 100)) AND ((c2 = 6)) (6 rows) --- 3261,3267 ---- Output: array_agg(c1 ORDER BY c1 USING <^ NULLS LAST), c2 Group Key: ft2.c2 -> Foreign Scan on public.ft2 ! Output: c2, c1 Remote SQL: SELECT "C 1", c2 FROM "S 1"."T 1" WHERE (("C 1" < 100)) AND ((c2 = 6)) (6 rows) *** a/src/backend/optimizer/plan/planner.c --- b/src/backend/optimizer/plan/planner.c *************** *** 237,243 **** static void apply_scanjoin_target_to_paths(PlannerInfo *root, List *scanjoin_targets, List *scanjoin_targets_contain_srfs, bool scanjoin_target_parallel_safe, ! bool tlist_same_exprs); static void create_partitionwise_grouping_paths(PlannerInfo *root, RelOptInfo *input_rel, RelOptInfo *grouped_rel, --- 237,244 ---- List *scanjoin_targets, List *scanjoin_targets_contain_srfs, bool scanjoin_target_parallel_safe, ! bool tlist_same_exprs, ! bool modify_in_place); static void create_partitionwise_grouping_paths(PlannerInfo *root, RelOptInfo *input_rel, RelOptInfo *grouped_rel, *************** *** 2046,2052 **** grouping_planner(PlannerInfo *root, bool inheritance_update, apply_scanjoin_target_to_paths(root, current_rel, scanjoin_targets, scanjoin_targets_contain_srfs, scanjoin_target_parallel_safe, ! scanjoin_target_same_exprs); /* * Save the various upper-rel PathTargets we just computed into --- 2047,2053 ---- apply_scanjoin_target_to_paths(root, current_rel, scanjoin_targets, scanjoin_targets_contain_srfs, scanjoin_target_parallel_safe, ! scanjoin_target_same_exprs, true); /* * Save the various upper-rel PathTargets we just computed into *************** *** 6899,6905 **** apply_scanjoin_target_to_paths(PlannerInfo *root, List *scanjoin_targets, List *scanjoin_targets_contain_srfs, bool scanjoin_target_parallel_safe, ! bool tlist_same_exprs) { ListCell *lc; PathTarget *scanjoin_target; --- 6900,6907 ---- List *scanjoin_targets, List *scanjoin_targets_contain_srfs, bool scanjoin_target_parallel_safe, ! bool tlist_same_exprs, ! bool modify_in_place) { ListCell *lc; PathTarget *scanjoin_target; *************** *** 6992,6999 **** apply_scanjoin_target_to_paths(PlannerInfo *root, scanjoin_target->sortgrouprefs; else { ! newpath = (Path *) create_projection_path(root, rel, subpath, ! scanjoin_target); lfirst(lc) = newpath; } } --- 6994,7014 ---- scanjoin_target->sortgrouprefs; else { ! /* ! * Don't use apply_projection_to_path() when modify_in_place is ! * false, because there could be other pointers to these paths, ! * and therefore we mustn't modify them in place. ! */ ! if (modify_in_place) ! newpath = (Path *) apply_projection_to_path(root, ! rel, ! subpath, ! scanjoin_target); ! else ! newpath = (Path *) create_projection_path(root, ! rel, ! subpath, ! scanjoin_target); lfirst(lc) = newpath; } } *************** *** 7068,7074 **** apply_scanjoin_target_to_paths(PlannerInfo *root, child_scanjoin_targets, scanjoin_targets_contain_srfs, scanjoin_target_parallel_safe, ! tlist_same_exprs); /* Save non-dummy children for Append paths. */ if (!IS_DUMMY_REL(child_rel)) --- 7083,7089 ---- child_scanjoin_targets, scanjoin_targets_contain_srfs, scanjoin_target_parallel_safe, ! tlist_same_exprs, false); /* Save non-dummy children for Append paths. */ if (!IS_DUMMY_REL(child_rel)) *** a/src/test/regress/expected/subselect.out --- b/src/test/regress/expected/subselect.out *************** *** 1255,1264 **** select * from x, x x2 where x.n = x2.n; QUERY PLAN ---------------------------------------------------------------------------- Result ! Output: subselect_tbl.f1, now(), subselect_tbl_1.f1, now() One-Time Filter: (now() = now()) -> Nested Loop ! Output: subselect_tbl.f1, subselect_tbl_1.f1 -> Seq Scan on public.subselect_tbl Output: subselect_tbl.f1, subselect_tbl.f2, subselect_tbl.f3 -> Materialize --- 1255,1264 ---- QUERY PLAN ---------------------------------------------------------------------------- Result ! Output: subselect_tbl.f1, (now()), subselect_tbl_1.f1, (now()) One-Time Filter: (now() = now()) -> Nested Loop ! Output: subselect_tbl.f1, now(), subselect_tbl_1.f1, now() -> Seq Scan on public.subselect_tbl Output: subselect_tbl.f1, subselect_tbl.f2, subselect_tbl.f3 -> Materialize *** a/src/test/regress/expected/update.out --- b/src/test/regress/expected/update.out *************** *** 172,189 **** EXPLAIN (VERBOSE, COSTS OFF) UPDATE update_test t SET (a, b) = (SELECT b, a FROM update_test s WHERE s.a = t.a) WHERE CURRENT_USER = SESSION_USER; ! QUERY PLAN ! ------------------------------------------------------------------ Update on public.update_test t -> Result ! Output: $1, $2, t.c, (SubPlan 1 (returns $1,$2)), t.ctid One-Time Filter: (CURRENT_USER = SESSION_USER) -> Seq Scan on public.update_test t ! Output: t.c, t.a, t.ctid ! SubPlan 1 (returns $1,$2) ! -> Seq Scan on public.update_test s ! Output: s.b, s.a ! Filter: (s.a = t.a) (10 rows) UPDATE update_test t --- 172,189 ---- UPDATE update_test t SET (a, b) = (SELECT b, a FROM update_test s WHERE s.a = t.a) WHERE CURRENT_USER = SESSION_USER; ! QUERY PLAN ! ------------------------------------------------------------------------ Update on public.update_test t -> Result ! Output: ($1), ($2), t.c, ((SubPlan 1 (returns $1,$2))), t.ctid One-Time Filter: (CURRENT_USER = SESSION_USER) -> Seq Scan on public.update_test t ! Output: $1, $2, t.c, (SubPlan 1 (returns $1,$2)), t.ctid ! SubPlan 1 (returns $1,$2) ! -> Seq Scan on public.update_test s ! Output: s.b, s.a ! Filter: (s.a = t.a) (10 rows) UPDATE update_test t