pgsql: Expand AclMode to 64 bits

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

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

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&amp;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

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&amp;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&amp;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&amp;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&gt;
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