On Thu, Jun 16, 2016 at 9:26 AM, Robert Haas <robertmh...@gmail.com> wrote: > > On Tue, Jun 14, 2016 at 12:18 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: > > Amit Kapila <amit.kapil...@gmail.com> writes: > >> On Mon, Jun 13, 2016 at 9:37 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: > >>> ... I got a core dump in the window.sql test: > >>> which I think may be another manifestation of the failure-to-apply-proper- > >>> pathtarget issue we're looking at in this thread. Or maybe it's just > >>> an unjustified assumption in make_partialgroup_input_target that the > >>> input path must always have some sortgrouprefs assigned. > > > >> I don't see this problem after your recent commit > >> - 89d53515e53ea080029894939118365b647489b3. Are you still getting it? > > > > No. I am suspicious that there's still a bug there, ie we're looking at > > the wrong pathtarget; but the regression test doesn't actually crash. > > That might only be because we don't choose the parallelized path. > > OK, so there are quite a number of separate things here: > > 1. The case originally reported by Thomas Munro still fails. To fix > that, we probably need to apply scanjoin_target to each partial path. > But we can only do that if it's parallel-safe. It seems like what we > want is something like this: (1) During scan/join planning, somehow > skip calling generate_gather_paths for the topmost scan/join rel as we > do to all the others. (2) If scanjoin_target is not parallel-safe, > build a path for the scan/join phase that applies a Gather node to the > cheapest path and does projection at the Gather node. Then forget all > the partial paths so we can't do any bogus upper planning. (3) If > scanjoin_target is parallel-safe, replace the list of partial paths > for the topmost scan/join rel with a new list where scanjoin_target > has been applied to each one. I haven't tested this so I might be > totally off-base about what's actually required here... >
I think we can achieve it just by doing something like what you have mentioned in (2) and (3). I am not sure if there is a need to skip generation of gather paths for top scan/join node. Please see the patch attached. I have just done some minimal testing to ensure that problem reported by Thomas Munro in this thread is fixed and verified that the fix is sane for problems [1][2] reported by sqlsmith. If you think this is on right lines, I can try to do more verification and try to add tests. > 2. In https://www.postgresql.org/message-id/15695.1465827...@sss.pgh.pa.us > Tom raised the issue that we need some test cases covering this area. > > 3. In https://www.postgresql.org/message-id/25521.1465837...@sss.pgh.pa.us > Tom proposed adding a GUC to set the minimum relation size required > for consideration of parallelism. > I have posted a patch for this upthread [3]. The main thing to verify is the default value of guc. [1] - https://www.postgresql.org/message-id/CAA4eK1Ky2=HsTsT4hmfL=eal5rv0_t59tvwzvk9hqkvn6do...@mail.gmail.com [2] - https://www.postgresql.org/message-id/CAFiTN-vzg5BkK6kAh3OMhvgRu-uJvkjz47ybtopMAfGJp=z...@mail.gmail.com [3] - https://www.postgresql.org/message-id/CA%2BkptmAU4xkzBpd8tie3X6o9_tE2oKm-0kDn8%2BZOF%3D2_qOMZNA%40mail.gmail.com With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
fix_scanjoin_pathtarget_v1.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
