The patch is in context format, includes docs and additional
regression tests, applies cleanly, passes "world" regression tests.
The parser changes don't cause any "backing up".
Discussion on the list seems to have reached a consensus that this
patch implements a useful feature. A previous version was reviewed.
This version attempts to respond to points previously raised.
Overall, it looked good, but there were a few things I would like Yeb
to address before mark it is marked ready for committer. I don't
think any of these will take long.
(1) Doc changes:
(a) These contain some unnecessary whitespace changes which make it
harder to see exactly what changed.
(b) There's one point where "cursors" should be possessive
"cursor's".
(c) I think it would be less confusing to be consistent about
mentioning "positional" and "named" in the same order each
time. Mixing it up like that is harder to read, at least for
me.
(2) The error for mixing positional and named parameters should be
moved up. That seems more important than "too many arguments" or
"provided multiple times" if both are true.
(3) The "-- END ADDITIONAL TESTS" comment shouldn't be added to the
regression tests.
The way this parses out the named parameters and then builds the
positional list, with some code from read_sql_construct() repeated in
read_cursor_args() seems a little bit clumsy to me, but I couldn't
think of a better way to do it.
-Kevin
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
