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

Hi,

I have reviewed the patch.

Patch applies cleanly. make/"make install"/"make check" all are fine.

I too see a good performance in execution time when LIMIT is pushed with
cursor query in postgres_fdw at execution time

However here are few comments on the changes:

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.

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?

Apart from this patch look good to me.

Regards,
--
Jeevan Chalke
Principal Software Engineer, Product Development
EnterpriseDB Corporation
The Enterprise PostgreSQL Company