Re: PassDownLimitBound for ForeignScan/CustomScan [take-2]

On Thu, Nov 10, 2016 at 10:59 AM, Kouhei Kaigai <kaigai(at)ak(dot)jp(dot)nec(dot)com>
wrote:

> > On Tue, Nov 8, 2016 at 6:54 AM, Jeevan Chalke
> > <jeevan(dot)chalke(at)enterprisedb(dot)com> wrote:
> > > 1. ps_numTuples is declared as long, however offset and count members
> in
> > > LimitState struct and bound member in SortState struct is int64.
> However
> > > long on 32 bit machine may be 32 bits and thus I think tuples_needed
> which
> > > is long may have overflow hazards as it may store int64 + int64. I
> think
> > > ps_numTuples should be int64.
> >
> > I suggested long originally because that's what ExecutorRun() was
> > using at the time. It seems that it got changed to uint64 in
> > 23a27b039d94ba359286694831eafe03cd970eef, so I guess we should
> > probably use uint64.
> >
> > > 2. Robert suggested following in the previous discussion:
> > > "For example, suppose we add a new PlanState member "long
> > > numTuples" where 0 means that the number of tuples that will be needed
> > > is unknown (so that most node types need not initialize it), a
> > > positive value is an upper bound on the number of tuples that will be
> > > fetched, and -1 means that it is known for certain that we will need
> > > all of the tuples."
> > >
> > > We should have 0 for the default case so that we don't need to
> initialize it
> > > at most of the places. But I see many such changes in the patch. I
> think
> > > this is not possible here since 0 can be a legal user provided value
> which
> > > cannot be set as a default (default is all rows).
> > >
> > > However do you think, can we avoid that? Is there any other way so
> that we
> > > don't need every node having ps_numTuples to be set explicitly?
> >
> > +1.
> >
> I thought we have to distinguish a case if LIMIT 0 is supplied.
> However, in this case, ExecLimit() never goes down to the child nodes,
> thus, its ps_numTuples shall not be referenced anywhere.
>
> OK, I'll use uint64 for ps_numTuples, and 0 shall be a usual default
> value that means no specific number of rows are given.

Marked as "returned with feedback" in 2016-11 commitfest.
Please feel free to update the status when you submit the latest patch.

Regards,
Hari Babu
Fujitsu Australia