Re: [COMMITTERS] pgsql: Add new catalog called pg_init_privs

Add new catalog called pg_init_privs

This new catalog holds the privileges which the system was
initialized with at initdb time, along with any permissions set
by extensions at CREATE EXTENSION time. This allows pg_dump
(and any other similar use-cases) to detect when the privileges
set on initdb-created or extension-created objects have been
changed from what they were set to at initdb/extension-creation
time and handle those changes appropriately.

Reviews by Alexander Korotkov, Jose Luis Tallon

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/6c268df1276e9dd73e4d2cc89cf8787e8f186bda

Modified Files
--------------
doc/src/sgml/catalogs.sgml | 108 +++++++++++++++++++++
src/backend/catalog/Makefile | 2 +-
src/backend/catalog/aclchk.c | 149 +++++++++++++++++++++++++++++
src/backend/catalog/dependency.c | 46 ++++++++-
src/bin/initdb/initdb.c | 143 +++++++++++++++++++++++++++
src/include/catalog/indexing.h | 3 +
src/include/catalog/pg_init_privs.h | 101 +++++++++++++++++++
src/test/regress/expected/sanity_check.out | 1 +
8 files changed, 549 insertions(+), 4 deletions(-)

Stephen Frost wrote:
> Add new catalog called pg_init_privs
>
> This new catalog holds the privileges which the system was
> initialized with at initdb time, along with any permissions set
> by extensions at CREATE EXTENSION time. This allows pg_dump
> (and any other similar use-cases) to detect when the privileges
> set on initdb-created or extension-created objects have been
> changed from what they were set to at initdb/extension-creation
> time and handle those changes appropriately.

If you have an extension that's marked not relocatable and drop it, its
schema is left behind; trying to create the extension again, *crash*.

$ cat /pgsql/install/master/share/extension/crash--1.sql
grant usage on schema @extschema@ to public;

$ cat /pgsql/install/master/share/extension/crash.control
comment = 'crash'
default_version = '1'
relocatable = false
superuser = true
schema = 'crash'

alvherre=# create extension crash;
CREATE EXTENSION
alvherre=# drop extension crash;
DROP EXTENSION
alvherre=# \dn
Listado de esquemas
Nombre | Dueño
--------+----------
crash | alvherre
public | alvherre
(2 filas)

alvherre=# create extension crash;
el servidor ha cerrado la conexión inesperadamente
Probablemente se debe a que el servidor terminó de manera anormal
antes o durante el procesamiento de la petición.
La conexión al servidor se ha perdido. Intentando reiniciar: con éxito.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Alvaro,

* Alvaro Herrera (alvherre(at)2ndquadrant(dot)com) wrote:
> Stephen Frost wrote:
> > Add new catalog called pg_init_privs
> >
> > This new catalog holds the privileges which the system was
> > initialized with at initdb time, along with any permissions set
> > by extensions at CREATE EXTENSION time. This allows pg_dump
> > (and any other similar use-cases) to detect when the privileges
> > set on initdb-created or extension-created objects have been
> > changed from what they were set to at initdb/extension-creation
> > time and handle those changes appropriately.
>
> If you have an extension that's marked not relocatable and drop it, its
> schema is left behind; trying to create the extension again, *crash*.

Will take a look at this, though I'm just about to commit a fix that's
probably related (and addresses Pavel's issue). Basically, for reasons
unknown, I was calling systable_endscan() immediately after
systable_getnext(), which doesn't work when you want to use the tuple
you got back.

Thanks!

Stephen

Stephen Frost wrote:
> Alvaro,

> Will take a look at this, though I'm just about to commit a fix that's
> probably related (and addresses Pavel's issue). Basically, for reasons
> unknown, I was calling systable_endscan() immediately after
> systable_getnext(), which doesn't work when you want to use the tuple
> you got back.

Yeah, I noticed that bug too. It might well explain the issue, because
the tuple is seen as 0x7f.

#0 heap_deform_tuple (tuple=tuple(at)entry=0x339f438, tupleDesc=tupleDesc(at)entry=0x7f680f3dcde0,
values=values(at)entry=0x3390cd8,
isnull=isnull(at)entry=0x339f3c8 "\177\177\177\177\177~\177\177h\367\071\003")
at /pgsql/source/master/src/backend/access/common/heaptuple.c:881
#1 0x0000000000479e3a in heap_modify_tuple (tuple=tuple(at)entry=0x339f438, tupleDesc=0x7f680f3dcde0,
replValues=replValues(at)entry=0x7ffd662a3770, replIsnull=replIsnull(at)entry=0x7ffd662a3750 "",
doReplace=doReplace(at)entry=0x7ffd662a3760 "")
at /pgsql/source/master/src/backend/access/common/heaptuple.c:817
#2 0x0000000000518feb in recordExtensionInitPriv (objoid=46960, classoid=2615, objsubid=0,
new_acl=0x339f188) at /pgsql/source/master/src/backend/catalog/aclchk.c:5305
#3 0x000000000051d2b5 in ExecGrant_Namespace (istmt=<optimized out>)
at /pgsql/source/master/src/backend/catalog/aclchk.c:2942

Not sure what "Pavel's issue" is, since it's not listed in the open
items page.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

* Alvaro Herrera (alvherre(at)2ndquadrant(dot)com) wrote:
> Stephen Frost wrote:
> > Alvaro,
>
> > Will take a look at this, though I'm just about to commit a fix that's
> > probably related (and addresses Pavel's issue). Basically, for reasons
> > unknown, I was calling systable_endscan() immediately after
> > systable_getnext(), which doesn't work when you want to use the tuple
> > you got back.
>
> Yeah, I noticed that bug too. It might well explain the issue, because
> the tuple is seen as 0x7f.
>
> #0 heap_deform_tuple (tuple=tuple(at)entry=0x339f438, tupleDesc=tupleDesc(at)entry=0x7f680f3dcde0,
> values=values(at)entry=0x3390cd8,
> isnull=isnull(at)entry=0x339f3c8 "\177\177\177\177\177~\177\177h\367\071\003")
> at /pgsql/source/master/src/backend/access/common/heaptuple.c:881
> #1 0x0000000000479e3a in heap_modify_tuple (tuple=tuple(at)entry=0x339f438, tupleDesc=0x7f680f3dcde0,
> replValues=replValues(at)entry=0x7ffd662a3770, replIsnull=replIsnull(at)entry=0x7ffd662a3750 "",
> doReplace=doReplace(at)entry=0x7ffd662a3760 "")
> at /pgsql/source/master/src/backend/access/common/heaptuple.c:817
> #2 0x0000000000518feb in recordExtensionInitPriv (objoid=46960, classoid=2615, objsubid=0,
> new_acl=0x339f188) at /pgsql/source/master/src/backend/catalog/aclchk.c:5305
> #3 0x000000000051d2b5 in ExecGrant_Namespace (istmt=<optimized out>)
> at /pgsql/source/master/src/backend/catalog/aclchk.c:2942
>
> Not sure what "Pavel's issue" is, since it's not listed in the open
> items page.

Here's the latest message ID on the thread he started.

CAFj8pRB_8WggxG1EKgfDQ_G_C1p1iLC_j9M3JfLfMLd2Vxt_+w(at)mail(dot)gmail(dot)com

Pretty sure it's the same issue. Going through testing now and will
push the fix very shortly.

Thanks!

Stephen

Stephen Frost wrote:

> Here's the latest message ID on the thread he started.
>
> CAFj8pRB_8WggxG1EKgfDQ_G_C1p1iLC_j9M3JfLfMLd2Vxt_+w(at)mail(dot)gmail(dot)com
>
> Pretty sure it's the same issue. Going through testing now and will
> push the fix very shortly.

May I suggest adding the extension I showed to
src/test/modules/test_extensions.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Alvaro,

On Friday, April 15, 2016, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> wrote:

> Stephen Frost wrote:
>
> > Here's the latest message ID on the thread he started.
> >
> > CAFj8pRB_8WggxG1EKgfDQ_G_C1p1iLC_j9M3JfLfMLd2Vxt_+w(at)mail(dot)gmail(dot)com
> <javascript:;>
> >
> > Pretty sure it's the same issue. Going through testing now and will
> > push the fix very shortly.
>
> May I suggest adding the extension I showed to
> src/test/modules/test_extensions.
>

Yup, already done and will be included in my commit.

Thanks!

Stephen

* Alvaro Herrera (alvherre(at)2ndquadrant(dot)com) wrote:
> Stephen Frost wrote:
> > Alvaro,
>
> > Will take a look at this, though I'm just about to commit a fix that's
> > probably related (and addresses Pavel's issue). Basically, for reasons
> > unknown, I was calling systable_endscan() immediately after
> > systable_getnext(), which doesn't work when you want to use the tuple
> > you got back.
>
> Yeah, I noticed that bug too. It might well explain the issue, because
> the tuple is seen as 0x7f.

Fix pushed.

Thanks!

Stephen