29.07.2019 18:37, Stephen Frost wrote:
Greetings,
* Tom Lane (t...@sss.pgh.pa.us) wrote:
Bruce Momjian <br...@momjian.us> writes:
On Thu, Jul 18, 2019 at 06:53:12PM +0300, Anastasia Lubennikova wrote:
pg_upgrade from 9.6 fails if old cluster had non-standard ACL
on pg_catalog functions that have changed between versions,
for example pg_stop_backup(boolean).
Uh, wouldn't this affect any default-installed function where the
permission are modified? Is fixing only a few functions really helpful?
No, it's just functions whose signatures have changed enough that
a GRANT won't find them. I think the idea is that the set of
potentially-affected functions is determinate. I have to say that
the proposed patch seems like a complete kluge, though. For one
thing we'd have to maintain the list of affected functions in each
future release, and I have no faith in our remembering to do that.
Well, we aren't likely to do that ourselves, no, but perhaps we could
manage it with some prodding by having the buildfarm check for such
cases, not unlike how library maintainers check the ABI between versions
of the library they manage. Extension authors also deal with these
kinds of changes routinely when writing the upgrade scripts to go
between versions of their extension. I'm not convinced that this is a
great approach to go down either, to be clear. When going across major
versions, making people update their systems/code and re-test is
typically entirely reasonable to me.
Whatever we choose to do, we need to keep a list of changed functions. I
don't
think that it will add too much extra work to maintaining other catalog
changes
such as adding or renaming columns.
What's more, we must mention changed functions in migration release
notes. I've
checked documentation [1] and found out, that function API changes are not
described properly.
I think it is an important omission, so I attached the patch for
documentation.
Not quite sure, how many users have already migrated to version 10, still, I
believe it will help many others.
Suppressing the GRANT also seems reasonable for the case of objects
which have been renamed- clearly whatever is using those functions is
going to have to be modified to deal with the new name of the function,
requiring that the GRANT be re-issued doesn't seem like it's that much
more to ask of users. On the other hand, properly written tools that
check the version of PG and use the right function names could possibly
"just work" following a major version upgrade, if the privilege was
brought across to the new major version correctly.
That's exactly the case.
We also don't want to mistakenly GRANT users more access then they
should have though- if pg_stop_backup() one day grows an
optional argument to run some server-side script, I don't think we'd
want to necessairly just give access to that ability to roles who,
today, can execute the current pg_stop_backup() function. Of course, if
we added such a capability, hopefully we would do so in a way that less
privileged roles could continue to use the existing capability without
having access to run such a server-side script.
I also don't think that the current patch is actually sufficient to deal
with all the changes we've made between the versions- what about column
names on catalog tables/views that were removed, or changed/renamed..?
I can't get the problem you describe here. As far as I understand, various
changes of catalog tables and views are already handled correctly in
pg_upgrade.
In an ideal world, it seems like we'd make a judgement call and arrange
to pull the privileges across when we can do so without granting the
user privileges beyond those that were intended, and otherwise we'd
suppress the GRANT to avoid getting an error. I'm not sure what a good
way is to go about either figuring out a way to pull the privileges
across, or to suppress the GRANTs when we need to (or always), would be
though. Neither seems easy to solve in a clean way.
Certainly open to suggestions.
Based on our initial bug report, I would vote against suppressing the
GRANTS,
because it leads to an unexpected failure in case a user has a special
role for
use in a third-party utility such as a backup tool, which already took
care of
internal API changes.
Still I agree with your arguments about possibility of providing more grants
than expected. Ideally, we do not change behaviour of existing functions
that
much, but in real-world it may happen.
Maybe, as a compromise, we can reset grants to default for all changed
functions
and also generate a script that will allow a user to preserve privileges
of the
old cluster by analogy with analyze_new_cluster script.
What do you think?
[1] https://www.postgresql.org/docs/10/release-10.html
--
Anastasia Lubennikova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
diff --git a/doc/src/sgml/release-10.sgml b/doc/src/sgml/release-10.sgml
index 7b2130e3c1..1b685b44da 100644
--- a/doc/src/sgml/release-10.sgml
+++ b/doc/src/sgml/release-10.sgml
@@ -9230,6 +9230,32 @@ Branch: REL_10_STABLE [5159626af] 2017-11-03 14:14:16 -0400
</para>
</listitem>
+ <listitem>
+<!--
+2017-03-22 [017e4f258] Expose waitforarchive option through pg_stop_backup()
+2017-08-05 [52f8a59dd] Make pg_stop_backup's wait_for_archive flag work on stan
+-->
+ <para>
+ Add an optional <literal>wait_for_archive</literal> argument to the <link
+ linkend="functions-admin"><function>pg_stop_backup()</function></link>
+ function to allow waiting for all <acronym>WAL</acronym> to be archived (David Steele)
+ </para>
+ </listitem>
+
+ <listitem>
+<!--
+2016-12-12 [a924c327e] Add support for temporary replication slots
+-->
+ <para>
+ Add an optional <literal>temporary</literal> argument to
+ <literal>pg_create_logical_replication_slot()</literal>
+ and <literal>pg_create_physical_replication_slot()</literal> functions
+ to allow creation of <link
+ linkend="functions-replication-table">temporary replication slots</link>
+ (Petr Jelinek)
+ </para>
+ </listitem>
+
</itemizedlist>
</sect2>
