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. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
