Re: ERROR: ORDER/GROUP BY expression not found in targetlist

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Amit Kapila <amit(dot)kapila(at)enterprisedb(dot)com>, David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: ERROR: ORDER/GROUP BY expression not found in targetlist
Date: 2016-06-16 12:00:36
Message-ID: CAA4eK1+tn1hvFM_E31Gxxh+G9sWxcTecmkXYw0EGTvD4eM6aPg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Jun 16, 2016 at 9:26 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
> On Tue, Jun 14, 2016 at 12:18 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> writes:
> >> On Mon, Jun 13, 2016 at 9:37 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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.1465827695@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.1465837597@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_t59tvWzVK9HQKvN6Dovkw@mail.gmail.com
[2] -
https://www.postgresql.org/message-id/CAFiTN-vzg5BkK6kAh3OMhvgRu-uJvkjz47ybtopMAfGJp=zWqA@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

Attachment Content-Type Size
fix_scanjoin_pathtarget_v1.patch application/octet-stream 5.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Magnus Hagander 2016-06-16 12:05:39 pg_upgrade vs vacuum_cost_delay
Previous Message Martín Marqués 2016-06-16 11:37:45 Re: [GENERAL] PgQ and pg_dump