On 3/9/22 18:06, Nathan Bossart wrote:
On Wed, Mar 09, 2022 at 06:11:19PM -0500, Chapman Flack wrote:
I think the listitem
In the same connection as before, issue the command:
SELECT * FROM pg_backup_stop(true);
would be clearer if it used named-parameter form, wait_for_archive => true.
This is not strictly necessary, of course, for a function with a single
IN parameter, but it's good documentation (and also could save us headaches
like these if there is ever another future need to give it more parameters).
That listitem doesn't say anything about what the parameter means, which
is a little weird, but probably ok because the next listitem does go into
it in some detail. I don't think a larger reorg is needed to bring that
language one listitem earlier. Just naming the parameter is probably
enough to make it less puzzling (or adding in that listitem, at most,
"the effect of the wait_for_archive parameter is explained below").
For consistency (and the same futureproofing benefit), I'd go to
fast => false in the earlier pg_backup_start as well.
I'm more ambivalent about label => 'label'. It would be consistent,
but should we just agree for conciseness that there will always be
a label and it will always be first?
You can pretty much tell in a call what's a label; it's those anonymous
trues and falses that are easier to read with named notation.
Done. I went ahead and added "label => 'label'" for consistency.
+1 from me.
Regards,
-David
