Re: ERROR: ORDER/GROUP BY expression not found in targetlist - Mailing list pgsql-hackers
| From | Amit Kapila |
|---|---|
| Subject | Re: ERROR: ORDER/GROUP BY expression not found in targetlist |
| Date | |
| Msg-id | CAA4eK1+tn1hvFM_E31Gxxh+G9sWxcTecmkXYw0EGTvD4eM6aPg@mail.gmail.com Whole thread Raw |
| In response to | Re: ERROR: ORDER/GROUP BY expression not found in targetlist (Robert Haas <robertmhaas@gmail.com>) |
| Responses |
Re: ERROR: ORDER/GROUP BY expression not found in targetlist
(Robert Haas <robertmhaas@gmail.com>)
Re: ERROR: ORDER/GROUP BY expression not found in targetlist (Tom Lane <tgl@sss.pgh.pa.us>) |
| List | pgsql-hackers |
On Thu, Jun 16, 2016 at 9:26 AM, Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Tue, Jun 14, 2016 at 12:18 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > Amit Kapila <amit.kapila16@gmail.com> writes:
> >> On Mon, Jun 13, 2016 at 9:37 PM, Tom Lane <tgl@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...
>
>
> On Tue, Jun 14, 2016 at 12:18 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > Amit Kapila <amit.kapila16@gmail.com> writes:
> >> On Mon, Jun 13, 2016 at 9:37 PM, Tom Lane <tgl@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.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.
Attachment
pgsql-hackers by date: