Hello all, as this is my first mail to pgsql-hackers, please be gentle :)
I've looked at the patch, and as I'm not that familiar with the pg-sourcecode, customs and so on, this isn't a review, more like food for thought and all should be taken with a grain of salt. :) So here are a few questions and remarks: >+ double limit_tuples = -1.0; Surely the limit cannot be fractional, and must be an integer. So wouldn't it be better the same type as say: >+ if (root->limit_tuples >= 0.0 && Than you could also compare with ">= 0", not ">= 0.0". node->ss.ps.ps_numTuples is f.i. an uint64. Or is there a specific reason the limit must be a double? And finally: >+ if (node->ss.ps.ps_numTuples > 0) >+ appendStringInfo(&buf, " LIMIT %ld", node->ss.ps.ps_numTuples); vs. >+ appendStringInfo(&buf, "%s LIMIT %lu", >+ sql, >node->ss.ps.ps_numTuples); It seems odd to have two different format strings here for the same variable. A few comments miss "." at the end, like these: >+ * Also, pass down the required number of tuples >+ * Pass down the number of required tuples by the upper node And this comment might be better "were we already called?" >+ bool rs_started; /* are we already called? */ Hope this helps, and thank you for working on this issue. Regards, Tels -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
