Thread: pgsql: Expand AclMode to 64 bits
Expand AclMode to 64 bits We're running out of bits for new permissions. This change doubles the number of permissions we can accomodate from 16 to 32, so the forthcoming new ones for vacuum/analyze don't exhaust the pool. Nathan Bossart Reviewed by: Bharath Rupireddy, Kyotaro Horiguchi, Stephen Frost, Robert Haas, Mark Dilger, Tom Lane, Corey Huinker, David G. Johnston, Michael Paquier. Discussion: https://postgr.es/m/20220722203735.GB3996698@nathanxps13 Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/7b378237aa805711353075de142021b1d40ff3b0 Modified Files -------------- src/backend/nodes/outfuncs.c | 2 +- src/bin/pg_upgrade/check.c | 35 +++++++++++++++++++++++++++++++++++ src/include/catalog/catversion.h | 2 +- src/include/catalog/pg_type.dat | 4 ++-- src/include/nodes/parsenodes.h | 6 +++--- src/include/utils/acl.h | 28 ++++++++++++++-------------- 6 files changed, 56 insertions(+), 21 deletions(-)
Hi Andrew, On Wed, Nov 23, 2022 at 07:44:04PM +0000, Andrew Dunstan wrote: > Expand AclMode to 64 bits > > We're running out of bits for new permissions. This change doubles the > number of permissions we can accomodate from 16 to 32, so the > forthcoming new ones for vacuum/analyze don't exhaust the pool. > > Nathan Bossart > > Reviewed by: Bharath Rupireddy, Kyotaro Horiguchi, Stephen Frost, Robert > Haas, Mark Dilger, Tom Lane, Corey Huinker, David G. Johnston, Michael > Paquier. crake is complaining for the upgrades from v12: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=crake&dt=2022-11-23%2023%3A32%3A04 It seems that there are some tables dependent on aclitem, bumping on your incompatible change. -- Michael
Attachment
On 2022-11-23 We 19:40, Michael Paquier wrote: > Hi Andrew, > > On Wed, Nov 23, 2022 at 07:44:04PM +0000, Andrew Dunstan wrote: >> Expand AclMode to 64 bits >> >> We're running out of bits for new permissions. This change doubles the >> number of permissions we can accomodate from 16 to 32, so the >> forthcoming new ones for vacuum/analyze don't exhaust the pool. >> >> Nathan Bossart >> >> Reviewed by: Bharath Rupireddy, Kyotaro Horiguchi, Stephen Frost, Robert >> Haas, Mark Dilger, Tom Lane, Corey Huinker, David G. Johnston, Michael >> Paquier. > crake is complaining for the upgrades from v12: > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=crake&dt=2022-11-23%2023%3A32%3A04 > > It seems that there are some tables dependent on aclitem, bumping on > your incompatible change. Yeah, testing a fix for it, thanks. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
On Thu, 24 Nov 2022 at 08:44, Andrew Dunstan <andrew@dunslane.net> wrote: > Expand AclMode to 64 bits I noticed this causes a few new warnings on MSVC: acl.c(629): warning C4334: '<<': result of 32-bit shift implicitly converted to 64 bits (was 64-bit shift intended?) acl.c(631): warning C4334: '<<': result of 32-bit shift implicitly converted to 64 bits (was 64-bit shift intended?) acl.c(1789): warning C4334: '<<': result of 32-bit shift implicitly converted to 64 bits (was 64-bit shift intended?) The attached fixes. I'll push this shortly. David
Attachment
Hi Andrew, On Thu, Nov 24, 2022 at 10:18 AM Andrew Dunstan <andrew@dunslane.net> wrote: > On 2022-11-23 We 19:40, Michael Paquier wrote: > > Hi Andrew, > > > > On Wed, Nov 23, 2022 at 07:44:04PM +0000, Andrew Dunstan wrote: > >> Expand AclMode to 64 bits > >> > >> We're running out of bits for new permissions. This change doubles the > >> number of permissions we can accomodate from 16 to 32, so the > >> forthcoming new ones for vacuum/analyze don't exhaust the pool. > >> > >> Nathan Bossart > >> > >> Reviewed by: Bharath Rupireddy, Kyotaro Horiguchi, Stephen Frost, Robert > >> Haas, Mark Dilger, Tom Lane, Corey Huinker, David G. Johnston, Michael > >> Paquier. > > crake is complaining for the upgrades from v12: > > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=crake&dt=2022-11-23%2023%3A32%3A04 > > > > It seems that there are some tables dependent on aclitem, bumping on > > your incompatible change. > > Yeah, testing a fix for it, thanks. Not sure if it is related to the above, but I noticed a problem when rebasing my patch that moves requiredPerms out of RangeTblEntry. I think the commit may have missed doing the following (diff attached): diff --git a/src/backend/nodes/gen_node_support.pl b/src/backend/nodes/gen_node_support.pl index d3f25299de..b6f086e262 100644 --- a/src/backend/nodes/gen_node_support.pl +++ b/src/backend/nodes/gen_node_support.pl @@ -954,7 +954,6 @@ _read${n}(void) } elsif ($t eq 'uint32' || $t eq 'bits32' - || $t eq 'AclMode' || $t eq 'BlockNumber' || $t eq 'Index' || $t eq 'SubTransactionId') @@ -962,7 +961,8 @@ _read${n}(void) print $off "\tWRITE_UINT_FIELD($f);\n"; print $rff "\tREAD_UINT_FIELD($f);\n" unless $no_read; } - elsif ($t eq 'uint64') + elsif ($t eq 'uint64' + || $t eq 'AclMode') { print $off "\tWRITE_UINT64_FIELD($f);\n"; print $rff "\tREAD_UINT64_FIELD($f);\n" unless $no_read; -- Thanks, Amit Langote EDB: http://www.enterprisedb.com
Attachment
On 2022-11-25 Fr 01:52, Amit Langote wrote: > Hi Andrew, > > On Thu, Nov 24, 2022 at 10:18 AM Andrew Dunstan <andrew@dunslane.net> wrote: >> On 2022-11-23 We 19:40, Michael Paquier wrote: >>> Hi Andrew, >>> >>> On Wed, Nov 23, 2022 at 07:44:04PM +0000, Andrew Dunstan wrote: >>>> Expand AclMode to 64 bits >>>> >>>> We're running out of bits for new permissions. This change doubles the >>>> number of permissions we can accomodate from 16 to 32, so the >>>> forthcoming new ones for vacuum/analyze don't exhaust the pool. >>>> >>>> Nathan Bossart >>>> >>>> Reviewed by: Bharath Rupireddy, Kyotaro Horiguchi, Stephen Frost, Robert >>>> Haas, Mark Dilger, Tom Lane, Corey Huinker, David G. Johnston, Michael >>>> Paquier. >>> crake is complaining for the upgrades from v12: >>> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=crake&dt=2022-11-23%2023%3A32%3A04 >>> >>> It seems that there are some tables dependent on aclitem, bumping on >>> your incompatible change. >> Yeah, testing a fix for it, thanks. > Not sure if it is related to the above, but I noticed a problem when > rebasing my patch that moves requiredPerms out of RangeTblEntry. I > think the commit may have missed doing the following (diff attached): pushed, thanks. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
On Sat, Nov 26, 2022 at 3:00 AM Andrew Dunstan <andrew@dunslane.net> wrote: > >>> crake is complaining for the upgrades from v12: > >>> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=crake&dt=2022-11-23%2023%3A32%3A04 > >>> > >>> It seems that there are some tables dependent on aclitem, bumping on > >>> your incompatible change. > pushed, thanks. crake is green, but three sparc boxes are red with the same sort of Xversion failure, I think?
On 2022-12-01 Th 19:54, Thomas Munro wrote: > On Sat, Nov 26, 2022 at 3:00 AM Andrew Dunstan <andrew@dunslane.net> wrote: >>>>> crake is complaining for the upgrades from v12: >>>>> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=crake&dt=2022-11-23%2023%3A32%3A04 >>>>> >>>>> It seems that there are some tables dependent on aclitem, bumping on >>>>> your incompatible change. >> pushed, thanks. > crake is green, but three sparc boxes are red with the same sort of > Xversion failure, I think? Yeah, there is a fix committed, see <https://github.com/PGBuildFarm/client-code/commit/a6149f413c9387c331b2744008706903ce171584> which crake and other animals have been testing. I will publish a new release very soon. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com