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