pgsql: Rewrite the code that applies scan/join targets to paths.

On 2018-03-29 19:52:40 +0000, Robert Haas wrote:

Rewrite the code that applies scan/join targets to paths.

If the toplevel scan/join target list is parallel-safe, postpone
generating Gather (or Gather Merge) paths until after the toplevel has
been adjusted to return it. This (correctly) makes queries with
expensive functions in the target list more likely to choose a
parallel plan, since the cost of the plan now reflects the fact that
the evaluation will happen in the workers rather than the leader.
The original complaint about this problem was from Jeff Janes.

If the toplevel scan/join relation is partitioned, recursively apply
the changes to all partitions. This sometimes allows us to get rid of
Result nodes, because Append is not projection-capable but its
children may be. It also cleans up what appears to be incorrect SRF
handling from commit e2f1eb0ee30d144628ab523432320f174a2c8966: the old
code had no knowledge of SRFs for child scan/join rels.

Because we now use create_projection_path() in some cases where we
formerly used apply_projection_to_path(), this changes the ordering
of columns in some queries generated by postgres_fdw. Update
regression outputs accordingly.

Patch by me, reviewed by Amit Kapila and by Ashutosh Bapat. Other
fixes for this problem (substantially different from this version)
were reviewed by Dilip Kumar, Amit Khandekar, and Marina Polyakova.

Discussion: /messages/by-id/CAMkU=1ycXNipvhWuweUVpKuyu6SpNjF=yHWu4c4US5JgVGxtZQ@mail.gmail.com

Not 100% sure it's this patch, but if not, it's also one of the ones you
committed ;)

There's a valgrind complaint:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=lousyjack&dt=2018-03-30%2002%3A03%3A01

Last file mtime in snapshot: Fri Mar 30 01:25:39 2018 GMT
===================================================
==6442== Invalid read of size 4
==6442== at 0x78D725: apply_scanjoin_target_to_paths (planner.c:6843)
==6442== by 0x7860F0: grouping_planner (planner.c:1995)
==6442== by 0x7847AF: subquery_planner (planner.c:966)
==6442== by 0x758144: set_subquery_pathlist (allpaths.c:2150)
==6442== by 0x755B37: set_rel_size (allpaths.c:378)
==6442== by 0x75594C: set_base_rel_sizes (allpaths.c:280)
==6442== by 0x7557F3: make_one_rel (allpaths.c:178)
==6442== by 0x783253: query_planner (planmain.c:259)
==6442== by 0x785D19: grouping_planner (planner.c:1866)
==6442== by 0x7847AF: subquery_planner (planner.c:966)
==6442== by 0x7835F6: standard_planner (planner.c:405)
==6442== by 0x783366: planner (planner.c:263)
==6442== by 0x865254: pg_plan_query (postgres.c:808)
==6442== by 0x86538E: pg_plan_queries (postgres.c:874)
==6442== by 0x86566C: exec_simple_query (postgres.c:1049)
==6442== by 0x869DE9: PostgresMain (postgres.c:4149)
==6442== by 0x7D3EFB: BackendRun (postmaster.c:4409)
==6442== by 0x7D35DF: BackendStartup (postmaster.c:4081)
==6442== by 0x7CFA7D: ServerLoop (postmaster.c:1754)
==6442== by 0x7CF00C: PostmasterMain (postmaster.c:1362)
==6442== Address 0x198e0528 is 62,696 bytes inside a recently re-allocated block of size 65,536 alloc'd
==6442== at 0x4C2FB6B: malloc (vg_replace_malloc.c:299)
==6442== by 0x9FB72E: AllocSetAlloc (aset.c:912)
==6442== by 0xA0582E: MemoryContextAllocZeroAligned (mcxt.c:855)
==6442== by 0x723E40: _copyValue (copyfuncs.c:4666)
==6442== by 0x7248C1: copyObjectImpl (copyfuncs.c:5061)
==6442== by 0x723CF4: _copyList (copyfuncs.c:4628)
==6442== by 0x7248D6: copyObjectImpl (copyfuncs.c:5068)
==6442== by 0x719A31: _copyAlias (copyfuncs.c:1206)
==6442== by 0x7243FF: copyObjectImpl (copyfuncs.c:4875)
==6442== by 0x71C66C: _copyRangeTblEntry (copyfuncs.c:2327)
==6442== by 0x7254ED: copyObjectImpl (copyfuncs.c:5520)
==6442== by 0x723CA7: _copyList (copyfuncs.c:4622)
==6442== by 0x7248D6: copyObjectImpl (copyfuncs.c:5068)
==6442== by 0x71E756: _copyQuery (copyfuncs.c:2962)
==6442== by 0x724915: copyObjectImpl (copyfuncs.c:5091)
==6442== by 0x757F0C: set_subquery_pathlist (allpaths.c:2044)
==6442== by 0x755B37: set_rel_size (allpaths.c:378)
==6442== by 0x75594C: set_base_rel_sizes (allpaths.c:280)
==6442== by 0x7557F3: make_one_rel (allpaths.c:178)
==6442== by 0x783253: query_planner (planmain.c:259)
==6442==
{
<insert_a_suppression_name_here>
Memcheck:Addr4
fun:apply_scanjoin_target_to_paths
fun:grouping_planner
fun:subquery_planner
fun:set_subquery_pathlist
fun:set_rel_size
fun:set_base_rel_sizes
fun:make_one_rel
fun:query_planner
fun:grouping_planner
fun:subquery_planner
fun:standard_planner
fun:planner
fun:pg_plan_query
fun:pg_plan_queries
fun:exec_simple_query
fun:PostgresMain
fun:BackendRun
fun:BackendStartup
fun:ServerLoop
fun:PostmasterMain
}

Greetings,

Andres Freund

On Fri, Mar 30, 2018 at 1:03 AM, Andres Freund <andres@anarazel.de> wrote:

Not 100% sure it's this patch, but if not, it's also one of the ones you
committed ;)

Looks like it.

There's a valgrind complaint:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=lousyjack&amp;dt=2018-03-30%2002%3A03%3A01

Last file mtime in snapshot: Fri Mar 30 01:25:39 2018 GMT
===================================================
==6442== Invalid read of size 4
==6442== at 0x78D725: apply_scanjoin_target_to_paths (planner.c:6843)

I've committed an attempt at a fix.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company