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

From: Andres Freund <andres(at)anarazel(dot)de>
To: Robert Haas <rhaas(at)postgresql(dot)org>
Cc: pgsql-committers(at)lists(dot)postgresql(dot)org
Subject: Re: pgsql: Rewrite the code that applies scan/join targets to paths.
Date: 2018-03-30 05:03:51
Message-ID: 20180330050351.bmxx4cdtz67czjda@alap3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

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

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Magnus Hagander 2018-03-30 10:35:47 pgsql: Fix typo in comment
Previous Message Bruce Momjian 2018-03-30 01:25:42 pgsql: docs: fix spacing around "if not exists" brackets

Browse pgsql-hackers by date

  From Date Subject
Next Message Kyotaro HORIGUCHI 2018-03-30 05:25:32 Re: Question about WalSndWriteData
Previous Message Amit Langote 2018-03-30 04:24:19 Re: [HACKERS] path toward faster partition pruning