From: Tom Lane <tgl@sss.pgh.pa.us>
To: Robert Haas <robertmhaas@gmail.com>
cc: Amit Kapila <amit.kapila16@gmail.com>, Noah Misch <noah@leadboat.com>,
        "Adam,
    Etienne (Nokia-TECH/Issy Les Moulineaux)" <etienne.adam@nokia.com>,
        PostgreSQL Bugs <pgsql-bugs@postgresql.org>,
        "Duquesne,
    Pierre (Nokia-TECH/Issy Les Moulineaux)" <pierre.duquesne@nokia.com>,
        pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: [HACKERS] [postgresql 10 beta3] unrecognized node type: 90
In-reply-to: 
 <CA+Tgmob-jDKc8DUTzsCsQEmX7JNqHQC9u3=059WaO95YLwoaAA@mail.gmail.com>
References: 
 <CAL7LBcYM-32B=n_-h_wPHWiML26UvqP3b_WxD7MgXgOwmxvTFw@mail.gmail.com>
 <11673.1502467154@sss.pgh.pa.us> <20170815025243.GA3986579@rfd.leadboat.com>
 <CAA4eK1KeQWZOoDmDmGMwuqzPW9JhRS+ditQVFdAfGjNmMZzqMQ@mail.gmail.com>
 <CA+TgmoaWPtfdcZjg+dqLephgKaG7kF8DEdvB870A2t4cUh5x+Q@mail.gmail.com>
 <26053.1502804769@sss.pgh.pa.us>
 <CAA4eK1J_dRv+pGt2bquwmwd_K9+Nght2=tbAc3uUzCinJTMmLg@mail.gmail.com>
 <2941.1502979564@sss.pgh.pa.us>
 <CAA4eK1KeGhogrkO1JWo=WnvEHm_i9tz6-Gt65zELB1hi=P28Hw@mail.gmail.com>
 <CAA4eK1+qD0yfh-iNDKrM7=qMxPOd2KkCZRTX+uxYsDMn0RBwUQ@mail.gmail.com>
 <28782.1502992247@sss.pgh.pa.us> <29467.1502993219@sss.pgh.pa.us>
 <CA+Tgmob+S=c=wqaJXNgKT2VhhAVRYFz=MagoTDWb0MueM1xBxg@mail.gmail.com>
 <18650.1502995942@sss.pgh.pa.us> <1105.1503008416@sss.pgh.pa.us>
 <24667.1503865781@sss.pgh.pa.us>
 <CAA4eK1LqHdSL0nH04EJ79AnUeKCHPf-rpgkrYszYpKETVFM-Wg@mail.gmail.com>
 <3924.1503923486@sss.pgh.pa.us>
 <CAA4eK1+eEZ5+4jMdWtGtAGK9kDHDcH7Q_xNSxXZV_0wwhTPbZQ@mail.gmail.com>
 <13540.1503931678@!
 sss.pgh.pa.us>
 <CA+TgmoabrmBs6WaJtnrkdyAnmcGUcNbsxm=H43Vs9Xj54BCWUA@mail.gmail.com>
 <16978.1503946802@sss.pgh.pa.us>
 <CA+Tgmob-jDKc8DUTzsCsQEmX7JNqHQC9u3=059WaO95YLwoaAA@mail.gmail.com>
Comments: In-reply-to Robert Haas <robertmhaas@gmail.com>
	message dated "Mon, 28 Aug 2017 18:03:42 -0400"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <32767.1503959717.1@sss.pgh.pa.us>
Date: Mon, 28 Aug 2017 18:35:17 -0400
Message-ID: <300.1503959717@sss.pgh.pa.us>
Precedence: bulk
Sender: pgsql-bugs-owner@postgresql.org

Robert Haas <robertmhaas@gmail.com> writes:
> On Mon, Aug 28, 2017 at 3:00 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Count the "that"s (you're failing to notice the next line).

> OK, true.  But "Academic literature" -> "The academic literature" is
> just second-guessing, I think.

No, it was more to avoid reflowing the paragraph (or leaving a weirdly
short line).

> There should never be a parallel_aware node that's not beneath a
> Gather or Gather Merge; I don't know what the meaning of such a plan
> would be, so I think we're safe against such a thing appearing in the
> future.  What I'm unclear about is what happens with nodes that aren't
> directly in the chain between the Gather and the parallel-aware node.

Nothing.  The parallel-aware node(s) get marked as dependent on the rescan
parameter, and then that marking bubbles up to their ancestor nodes (up
to the Gather).  Nodes that are not ancestral to any parallel-aware node
are unchanged.

> Now consider Parallel Hash
> (not yet committed), where we might get this:

> Something
> -> Gather
>   -> Merge Join
>     -> Sort
>       -> Parallel Seq Scan on a
>     -> Hash Join
>       -> Index Scan on b
>       -> Parallel Hash
>         -> Parallel Seq Scan on c

> Now what?  We clearly still need to force a re-sort of a, but what
> about the shared hash table built on c?  If we've still got that hash
> table and it was a single-batch join, there's probably no need to
> rescan it even though both the Parallel Hash node and the Parallel Seq
> Scan beneath it are parallel-aware.  In this case all workers
> collaborated to build a shared hash table covering all rows from c; if
> we've still got that, it's still good.  In fact, even the workers can
> reuse that hash table even though, for them, it'll not really be a
> re-scan at all.

Well, I'd say that's something for the parallel hash patch to resolve.
Yes, if the contents of the hash table are expected to be the same
regardless of how many workers were involved, then we shouldn't need
to rebuild it after a rescan of the Gather.  That would mean not marking
either Parallel Hash or its descendant Parallel Seq Scan as dependent
on the Gather's rescan param.  That's not terribly hard to mechanize
within the structure of this patch: just ignore the param at/below
the ParallelHash.  Cowboy coding would be, perhaps,

    if (plan->parallel_aware)
    {
        if (gather_param < 0)
            elog(ERROR, "parallel-aware plan node is not below a Gather");
+       if (IsA(plan, Hash))
+           gather_param = -1;
+       else
        context.paramids =
            bms_add_member(context.paramids, gather_param);
    }

but probably we should think of a more arm's-length way to do it.
Maybe parallel_aware should have more than two values, depending
on whether the result of the node is context-dependent or not.

			regards, tom lane


-- 
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs