Horton

Thread

Commits

  1. Don't call IS_DUMMY_REL() when cheapest_total_path might be junk.

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

  3. Implement partition-wise grouping/aggregation.

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

    Robert Haas <rhaas@postgresql.org> — 2018-03-29T19:52:40Z

    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: http://postgr.es/m/CAMkU=1ycXNipvhWuweUVpKuyu6SpNjF=yHWu4c4US5JgVGxtZQ@mail.gmail.com

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/11cf92f6e2e13c0a6e3f98be3e629e6bd90b74d5

Modified Files
--------------
contrib/postgres_fdw/expected/postgres_fdw.out |  81 ++-
src/backend/optimizer/plan/planner.c           | 282 ++++++---
src/test/regress/expected/partition_join.out   | 772 ++++++++++++-------------
3 files changed, 606 insertions(+), 529 deletions(-)

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

    Andres Freund <andres@anarazel.de> — 2018-03-30T05:03:51Z

    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: http://postgr.es/m/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

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

    Robert Haas <robertmhaas@gmail.com> — 2018-03-30T15:45:43Z

    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&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