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

Reply via email to