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
