Em ter., 13 de jul. de 2021 às 01:44, David Rowley <dgrowleyml@gmail.com> escreveu:
Thanks for having a look at this.
On Tue, 13 Jul 2021 at 11:04, Ranier Vilela <ranier.vf@gmail.com> wrote: >> 0001 Adds planner support for ORDER BY aggregates. > > /* Normal transition function without ORDER BY / DISTINCT. */ > Is it possible to avoid entering to initialize args if 'argno >= pertrans->numTransInputs'? > Like this: > > if (!pertrans->aggsortrequired && argno < pertrans->numTransInputs) > > And if argos is '>' that pertrans->numTransInputs? > The test shouldn't be, inside the loop? > > + /* > + * Don't initialize args for any ORDER BY clause that might > + * exist in a presorted aggregate. > + */ > + if (argno >= pertrans->numTransInputs) > + break;
The idea is to stop the loop before processing any Aggref arguments that might belong to the ORDER BY clause.
Yes, I get the idea.
We must still process other arguments up to the ORDER BY args though,
I may have misunderstood, but the other arguments are under pertrans->numTransInputs?
so we can't skip this loop.
The question not answered is if *argno* can '>=' that pertrans->numTransInputs,
before entering the loop?
If *can*, the loop might be useless in that case.
Note that we're doing argno++ inside the loop.
Another question is, if *argno* can '>' that pertrans->numTransInputs,
before the loop, the test will fail?
if (argno == pertrans->numTransInputs)
> I think that or can reduce the scope of variable 'sortlist' or simply remove it?
I've adjusted the scope of this. I didn't want to remove it because it's kinda useful to have it that way otherwise the 0002 patch would need to add it.
Nice.
>> 0002 is a WIP patch for DISTINCT support. This still lacks JIT >> support and I'm still not certain of the best where to store the >> previous value or tuple to determine if the current one is distinct >> from it. > > In the patch 0002, I think that can reduce the scope of variable 'aggstate'? > > + EEO_CASE(EEOP_AGG_PRESORTED_DISTINCT_SINGLE)
