Re: HashAgg's batching counter starts at 0, but Hash's starts at 1.

On Tue, 28 Jul 2020 at 15:01, Justin Pryzby <pryzby@telsasoft.com> wrote:
>
> On Tue, Jul 28, 2020 at 12:54:35PM +1200, David Rowley wrote:
> > On Mon, 27 Jul 2020 at 14:54, Justin Pryzby <pryzby@telsasoft.com> wrote:
> > > It's unrelated to hashAgg vs hashJoin, but I also noticed that this is shown
> > > only conditionally:
> > >
> > >         if (es->format != EXPLAIN_FORMAT_TEXT)
> > >         {
> > >                 if (es->costs && aggstate->hash_planned_partitions > 0)
> > >                 {
> > >                         ExplainPropertyInteger("Planned Partitions", NULL,
> > >                                                                    aggstate->hash_planned_partitions, es);
> > >
> > > That was conditional since it was introduced at 1f39bce02:
> > >
> > >         if (es->costs && aggstate->hash_planned_partitions > 0)
> > >         {
> > >                 ExplainPropertyInteger("Planned Partitions", NULL,
> > >                                                            aggstate->hash_planned_partitions, es);
> > >         }
> > >
> > > I think 40efbf870 should've handled this, too.
> >
> > hmm. I'm not sure. I think this should follow the same logic as what
> > "Disk Usage" follows, and right now we don't show Disk Usage unless we
> > spill.
>
> Huh ?  I'm referring to non-text format, which is what you changed, on the
> reasoning that the same plan *could* spill:

Oh, right. ... (Sudden bout of confusion due to lack of sleep)

Looks like it'll just need this line:

if (es->costs && aggstate->hash_planned_partitions > 0)

changed to:

if (es->costs)

I think we'll likely need to maintain not showing that property with
explain (costs off) as it'll be a bit more difficult to write
regression tests if we display it regardless of that option.

David