On Thu, Nov 10, 2016 at 10:59 AM, Kouhei Kaigai <kai...@ak.jp.nec.com> wrote:
> > On Tue, Nov 8, 2016 at 6:54 AM, Jeevan Chalke > > <jeevan.cha...@enterprisedb.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