On 2/28/22 23:51, Nathan Bossart wrote:
On Sat, Feb 26, 2022 at 02:06:14PM -0800, Nathan Bossart wrote:
On Sat, Feb 26, 2022 at 04:48:52PM +0000, Chapman Flack wrote:
Assuming the value is false, so no error is thrown, is it practical to determine
from flinfo->fn_expr whether the value was defaulted or supplied? If so, I would
further suggest reporting a deprecation WARNING if it was explicitly supplied,
with a HINT that the argument can simply be removed at the call site, and will
become unrecognized in some future release.
This is a good point. I think I agree with your proposed changes. I
believe it is possible to add a deprecation warning only when 'exclusive'
is specified. If anything, we can create a separate function that accepts
the 'exclusive' parameter and that always emits a NOTICE or WARNING.
I've spent some time looking into this, and I haven't found a clean way to
emit a WARNING only if the "exclusive" parameter is supplied (and set to
false). AFAICT flinfo->fn_expr doesn't tell us whether the parameter was
supplied or the default value was used. I was able to get it working by
splitting pg_start_backup() into 3 separate internal functions (i.e.,
pg_start_backup_1arg(), pg_start_backup_2arg(), and
pg_start_backup_3arg()), but this breaks calls such as
pg_start_backup('mylabel', exclusive => false), and it might complicate
privilege management for users.
Without a WARNING, I think it will be difficult to justify removing the
"exclusive" parameter in the future. We would either need to leave it
around forever, or we would have to risk unnecessarily breaking some
working backup scripts. I wonder if we should just remove it now and make
sure that this change is well-documented in the release notes.
Personally, I am in favor of removing it. We change/rename
functions/tables/views when we need to, and this happens in almost every
release.
What we need to do is make sure that an older installation won't
silently work in a broken way, i.e. if we remove the exclusive flag
somebody expecting the pre-9.6 behavior might not receive an error and
think everything is OK. That would not be good.
One option might be to rename the functions. Something like
pg_backup_start/stop.
Regards,
-David
