From: | Julien Rouhaud <rjuju123(at)gmail(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Andres Freund <andres(at)anarazel(dot)de>, thibaut(dot)madelaine(at)dalibo(dot)com, Robert Haas <robertmhaas(at)gmail(dot)com>, PostgreSQL mailing lists <pgsql-bugs(at)lists(dot)postgresql(dot)org> |
Subject: | Re: BUG #15669: Error with unnest in PG 11 (ERROR: 0A000) |
Date: | 2019-03-07 18:14:29 |
Message-ID: | CAOBaU_YBMTmwxBqMhmahdCHtT4YBDadoJUOwVdJRWWQNH-59cw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-bugs |
On Thu, Mar 7, 2019 at 4:52 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Julien Rouhaud <rjuju123(at)gmail(dot)com> writes:
> > This all looks good to me. I'm wondering about this chunk though:
>
> > + bool rel_is_partitioned = (rel->part_scheme && rel->part_rels);
>
> > IIUC it' safe for now (according to f069c91a579), but should we use
> > IS_PARTITIONED_REL macro instead? If yes, probably
> > create_ordinary_grouping_paths() should be updated too.
>
> Hmm, I was just copying the existing test in that function, but I agree
> that it seems pretty random to not be using the macro.
>
> Also, given that IS_PARTITIONED_REL is explicitly testing IS_DUMMY_REL,
> it doesn't really seem like we need the hack about forcing nparts etc
> to 0. I'd transferred that out of apply_scanjoin_target_to_paths into
> set_dummy_rel_pathlist and mark_dummy_rel, but that doesn't make it any
> less of an ugly kluge. In particular, that's not consistent with the
> idea that an appendrel automatically becomes dummy if we generate a
> zero-child path for it. So I'm now thinking we should drop that bit
> and instead make sure that everyplace that's testing for partitioned-ness
> is using this macro.
I did look for other usage yesterday, and AFAICT the only remaining
one is in create_ordinary_grouping_paths(), though it's already
checking for !IS_DUMMY_REL:
if (extra->patype != PARTITIONWISE_AGGREGATE_NONE &&
input_rel->part_scheme && input_rel->part_rels &&
!IS_DUMMY_REL(input_rel))
> One more thing --- after sleeping in it, I'm questioning my earlier
> feeling that apply_scanjoin_target_to_paths should flush existing paths
> for partitioned rels. Maybe it was done like that with the idea of
> letting paths with tlist computations done above the Append compete
> with paths doing the tlist computations below? That would be a fine
> idea if we had any costing factors that would produce a meaningful
> choice between the two; but I'm afraid that what we're probably getting
> right now is a quasi-random choice between paths whose costs differ
> only by rounding errors.
I don't know and the comments surely didn't mention that. But since
we're trying hard to speed up everything for high number of partitions
it seems like a good idea to avoid wasting cycles here if there's no
clear benefit.
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2019-03-07 18:40:10 | Re: BUG #15669: Error with unnest in PG 11 (ERROR: 0A000) |
Previous Message | PG Bug reporting form | 2019-03-07 17:02:41 | BUG #15676: FOR UPDATE is not allowed with UNION ALL (and of course with UNION/INTERSECT/EXCEPT, DISTINCT?) |