Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1qRHYK-0019qY-3g for pgsql-bugs@arkaria.postgresql.org; Wed, 02 Aug 2023 19:30:37 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.94.2) (envelope-from ) id 1qRHYH-007OgA-Iv for pgsql-bugs@arkaria.postgresql.org; Wed, 02 Aug 2023 19:30:33 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1qRHYF-007Og1-Tz for pgsql-bugs@lists.postgresql.org; Wed, 02 Aug 2023 19:30:33 +0000 Received: from out4-smtp.messagingengine.com ([66.111.4.28]) by makus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1qRHYB-000BTF-Hp for pgsql-bugs@lists.postgresql.org; Wed, 02 Aug 2023 19:30:31 +0000 Received: from compute6.internal (compute6.nyi.internal [10.202.2.47]) by mailout.nyi.internal (Postfix) with ESMTP id 6CC715C014D; Wed, 2 Aug 2023 15:30:25 -0400 (EDT) Received: from imap49 ([10.202.2.99]) by compute6.internal (MEProxy); Wed, 02 Aug 2023 15:30:25 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=eulerto.com; h= cc:cc:content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:sender :subject:subject:to:to; s=fm1; t=1691004625; x=1691091025; bh=PM U9WQ4iAVw0ERyE+dUnEFFAcj/kYiyuPR8hD7dTVsM=; b=FQOVQsZ/jXFwGjAOZ2 upovFN2RCiV+wIPGWSRcCvZrHfkTw5IzEiOy0cwfH8Hi1lcW9rGPjp1ZKXFaTTzc 253ZEHamrf+vKxaV7IPGx8lumkyWQTGOBhA+FtnEJQe1DuMYu1doP/ot6Nn/YJo9 FU0pJsK8eWONOPFwaKRTJCxxUqW0c+98BRM0rxxNfGQdMZ6kcS8WajWN+37BaZnr wSgGNVYqf6GnD8FtZ0POI4k4s3asHC/DHWQyxDl71WDdVcPX0RVSgyK3hQw8/RZq 2TQWyY7efQe8PSnGfUp20NcfklQnHtj9YLiiWVSDabYqiqkTvrC8NedViWfR98YD M38w== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:sender:subject :subject:to:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm3; t=1691004625; x=1691091025; bh=PMU9WQ4iAVw0E RyE+dUnEFFAcj/kYiyuPR8hD7dTVsM=; b=W54d34a3C5H4zelzOanORhXRlx9CN sVbUl5LCkp3bXirnJxrWsuEO8q7ucvbs9mExPcO0vtbC39m3l/1z1pIEGXXHl204 NWyu8bRL485Gd0DxJLKcZlORveCCvQsqidkWqU+kTH6Xe5q79GDi25+M+LoaHA6R M7R+6yHkL3j42tGfYVPLvfGXWAwIvh9r7Enda47Nz21QIXoTrEDyMh9OTXtHR4Y0 JvhjPZ9vQ9qt7ZjaHgaBHpHgY8W3grmSHAWwDUXUv+OOeQHojhjnaZhEYLIK6gMM w/oc25RXTBixGBfDMBkXfqSfHtt+78biNk82cTOz1817O4RPOuEWoKW4g== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedviedrkedtgdegudcutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecunecujfgurhepofgfggfkjghffffhvfevufgtsegrtd erreerredtnecuhfhrohhmpedfgfhulhgvrhcuvfgrvhgvihhrrgdfuceovghulhgvrhes vghulhgvrhhtohdrtghomheqnecuggftrfgrthhtvghrnhephedvveeuleejkeefleeihf effefgffegueefhfdvueegtddtgedvheeuvdfgudfgnecuffhomhgrihhnpehpohhsthhg rhgvshhqlhdrohhrghdpvghnthgvrhhprhhishgvuggsrdgtohhmnecuvehluhhsthgvrh fuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepvghulhgvrhesvghulhgvrhht ohdrtghomh X-ME-Proxy: Feedback-ID: i0c21471d:Fastmail Received: by mailuser.nyi.internal (Postfix, from userid 501) id 7F7F415A0091; Wed, 2 Aug 2023 15:30:24 -0400 (EDT) X-Mailer: MessagingEngine.com Webmail Interface User-Agent: Cyrus-JMAP/3.9.0-alpha0-624-g7714e4406d-fm-20230801.001-g7714e440 Mime-Version: 1.0 Message-Id: <90659bbc-e0cb-407d-b42b-5931346735ad@app.fastmail.com> In-Reply-To: References: <2427882.1690590240@sss.pgh.pa.us> Date: Wed, 02 Aug 2023 16:30:02 -0300 From: "Euler Taveira" To: "Kong Man" , "Tom Lane" Cc: "pgsql-bugs@lists.postgresql.org" Subject: Re: pg_restore 14 skips ACL COLUMN when --schema is used Content-Type: multipart/alternative; boundary=9ed1c71c70f44cd599122875703d07a8 List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --9ed1c71c70f44cd599122875703d07a8 Content-Type: text/plain On Mon, Jul 31, 2023, at 8:00 PM, Kong Man wrote: > In this example, I generate the public.event table's ToC entries for TABLE, ACL TABLE, and ACL COLUMN, then use the result on --use-list. pg_restore generates only the CREATE TABLE and GRANTs statements at the table, but not the column, level. You didn't provide a test case as requested but I investigated this issue according to your description. The following commands is sufficient to produce the analysis below. CREATE ROLE role1; CREATE ROLE role2; CREATE TABLE event ( col1 text, col2 text, col3 text ); GRANT SELECT ON TABLE event TO role1; GRANT SELECT, INSERT, UPDATE, DELETE ON TABLE event TO role2; GRANT SELECT (col1, col2) ON TABLE event TO role2; GRANT SELECT, INSERT, UPDATE (col3) ON TABLE event TO role2; I created these objects and ran pg_restore using a debugger. pg_dump -Fc -f /tmp/a.dump -d test pg_restore -l /tmp/a.dump > /tmp/a.toc gdb --args pg_restore --use-list=/tmp/a.toc -f - /tmp/a.dump I set a breakpoint to _tocEntryRequired function to inspect the TocEntry values. (gdb) b _tocEntryRequired After a few 'continue' and 'p *te' commands I got: (gdb) c Continuing. Breakpoint 1, _tocEntryRequired (te=0x5555555a6910, curSection=SECTION_PRE_DATA, AH=0x5555555a05b0) at pg_backup_archiver.c:2773 2773 } (gdb) p *te $8 = {prev = 0x5555555a67a0, next = 0x5555555a6be0, catalogId = {tableoid = 0, oid = 0}, dumpId = 3327, section = SECTION_NONE, hadDumper = false, tag = 0x555555585d30 "COLUMN event.col1", namespace = 0x5555555858b0 "public", tablespace = 0x0, tableam = 0x0, owner = 0x555555585340 "euler", desc = 0x555555585df0 "ACL", defn = 0x5555555a6a00 "GRANT SELECT(col1) ON TABLE public.event TO role2;\n", dropStmt = 0x0, copyStmt = 0x0, dependencies = 0x5555555a6a40, nDeps = 2, dataDumper = 0x0, dataDumperArg = 0x0, formatData = 0x555555585c70, dataLength = 0, reqs = 0, created = false, pending_prev = 0x0, pending_next = 0x0, depCount = 0, revDeps = 0x0, nRevDeps = 0, lockDeps = 0x0, nLockDeps = 0} (gdb) p te->dependencies[0] $18 = 214 (gdb) p *AH->tocsByDumpId[214] $17 = {prev = 0x5555555a64e0, next = 0x5555555a67a0, catalogId = {tableoid = 1259, oid = 183415}, dumpId = 214, section = SECTION_PRE_DATA, hadDumper = false, tag = 0x555555584e00 "event", namespace = 0x555555584f80 "public", tablespace = 0x555555585040 "", tableam = 0x555555584380 "heap", owner = 0x555555584440 "euler", desc = 0x555555584ec0 "TABLE", defn = 0x5555555a6740 "CREATE TABLE public.event (\n col1 text,\n col2 text,\n col3 text\n);\n", dropStmt = 0x5555555a61f0 "DROP TABLE public.event;\n", copyStmt = 0x0, dependencies = 0x0, nDeps = 0, dataDumper = 0x0, dataDumperArg = 0x0, formatData = 0x555555584d40, dataLength = 0, reqs = 1, created = false, pending_prev = 0x0, pending_next = 0x0, depCount = 0, revDeps = 0x0, nRevDeps = 0, lockDeps = 0x0, nLockDeps = 0} (gdb) p te->dependencies[1] $21 = 3326 (gdb) p *AH->tocsByDumpId[3326] $22 = {prev = 0x5555555a6650, next = 0x5555555a6910, catalogId = {tableoid = 0, oid = 0}, dumpId = 3326, section = SECTION_NONE, hadDumper = false, tag = 0x5555555854c0 "TABLE event", namespace = 0x555555585670 "public", tablespace = 0x0, tableam = 0x0, owner = 0x555555585730 "euler", desc = 0x555555585580 "ACL", defn = 0x5555555a6890 "GRANT SELECT ON TABLE public.event TO role1;\nGRANT SELECT,INSERT,DELETE,UPDATE ON TABLE public.event TO role2;\n", dropStmt = 0x0, copyStmt = 0x0, dependencies = 0x5555555a6050, nDeps = 1, dataDumper = 0x0, dataDumperArg = 0x0, formatData = 0x555555585400, dataLength = 0, reqs = 1, created = false, pending_prev = 0x0, pending_next = 0x0, depCount = 0, revDeps = 0x0, nRevDeps = 0, lockDeps = 0x0, nLockDeps = 0} It means that an ACL for columns has 2 dependencies (nDeps = 2) the function _tocEntryRequired() returns 0 (see code below). else if (ropt->schemaNames.head != NULL || ropt->schemaExcludeNames.head != NULL || ropt->selTypes) { /* * In a selective dump/restore, we want to restore these dependent * TOC entry types only if their parent object is being restored. * Without selectivity options, we let through everything in the * archive. Note there may be such entries with no parent, eg * non-default ACLs for built-in objects. * * This code depends on the parent having been marked already, * which should be the case; if it isn't, perhaps due to * SortTocFromFile rearrangement, skipping the dependent entry * seems prudent anyway. * * Ideally we'd handle, eg, table CHECK constraints this way too. * But it's hard to tell which of their dependencies is the one to * consult. */ if (te->nDeps != 1 || TocIDRequired(AH, te->dependencies[0]) == 0) return 0; } Hence, ProcessArchiveRestoreOptions() sets te->reqs to 0 that avoids the ACL for columns to be restored. te->reqs = _tocEntryRequired(te, curSection, AH); The discussion [1] in the commit 0d4e6ed3085 does not explain if the 'if' logic covers all cases. It certainly doesn't for the OP case. The only (hackish) suggestion I have ATM is to detect ACL for columns and returns 1. [1] https://www.postgresql.org/message-id/32668.1516848577%40sss.pgh.pa.us -- Euler Taveira EDB https://www.enterprisedb.com/ --9ed1c71c70f44cd599122875703d07a8 Content-Type: text/html Content-Transfer-Encoding: quoted-printable
On Mon, Ju= l 31, 2023, at 8:00 PM, Kong Man wrote:
In this example, I generate the public.event table's ToC entries for TA= BLE, ACL TABLE, and ACL COLUMN, then use the result on --use-list. = pg_restore generates only the CREATE TABLE and GRANTs statements at the= table, but not the column, level.

=
You didn't provide a test case as requested but I investigated this= issue
according to your description. The following comman= ds is sufficient to produce
the analysis below.
<= div>
CREATE ROLE role1;
CREATE ROLE role2;

CREATE TABLE event (
col1 text= ,
col2 text,
col3 text
);
<= /div>

GRANT SELECT ON TABLE event TO role1;
=
GRANT SELECT, INSERT, UPDATE, DELETE ON TABLE event TO role2;

GRANT SELECT (col1, col2) ON TABLE event TO role= 2;
GRANT SELECT, INSERT, UPDATE (col3) ON TABLE event TO r= ole2;

I created these objects and ran pg_re= store using a debugger.

pg_dump -Fc -f /tmp= /a.dump -d test
pg_restore -l /tmp/a.dump > /tmp/a.toc<= br>
gdb --args pg_restore --use-list=3D/tmp/a.toc -f - /tmp/a.= dump

I set a breakpoint to _tocEntryRequire= d function to inspect the TocEntry values.

= (gdb) b _tocEntryRequired

After a few 'cont= inue' and 'p *te' commands I got:

(gdb) c
Continuing.

Breakpoint 1, _toc= EntryRequired (te=3D0x5555555a6910, curSection=3DSECTION_PRE_DATA, AH=3D= 0x5555555a05b0) at pg_backup_archiver.c:2773
2773 }
(gdb) p *te
$8 =3D {prev =3D 0x5555555a67a0, next= =3D 0x5555555a6be0, catalogId =3D {tableoid =3D 0, oid =3D 0}, dumpId =3D= 3327, section =3D SECTION_NONE, hadDumper =3D false, 
  tag =3D 0x555555585d30 "COLUMN event.col1", namespace =3D 0x555= 5555858b0 "public", tablespace =3D 0x0, tableam =3D 0x0, owner =3D 0x555= 555585340 "euler", desc =3D 0x555555585df0 "ACL", 
&n= bsp; defn =3D 0x5555555a6a00 "GRANT SELECT(col1) ON TABLE public.event T= O role2;\n", dropStmt =3D 0x0, copyStmt =3D 0x0, dependencies =3D 0x5555= 555a6a40, nDeps =3D 2, dataDumper =3D 0x0, 
  da= taDumperArg =3D 0x0, formatData =3D 0x555555585c70, dataLength =3D 0, re= qs =3D 0, created =3D false, pending_prev =3D 0x0, pending_next =3D 0x0,= depCount =3D 0, revDeps =3D 0x0, nRevDeps =3D 0, 
&n= bsp; lockDeps =3D 0x0, nLockDeps =3D 0}
(gdb) p te->dep= endencies[0]
$18 =3D 214
(gdb) p *AH->toc= sByDumpId[214]
$17 =3D {prev =3D 0x5555555a64e0, next =3D = 0x5555555a67a0, catalogId =3D {tableoid =3D 1259, oid =3D 183415}, dumpI= d =3D 214, section =3D SECTION_PRE_DATA, hadDumper =3D false, 
<= /div>
  tag =3D 0x555555584e00 "event", namespace =3D 0x5555555= 84f80 "public", tablespace =3D 0x555555585040 "", tableam =3D 0x55555558= 4380 "heap", owner =3D 0x555555584440 "euler", 
 = ; desc =3D 0x555555584ec0 "TABLE", defn =3D 0x5555555a6740 "CREATE TABLE= public.event (\n    col1 text,\n    col2 = text,\n    col3 text\n);\n", 
  d= ropStmt =3D 0x5555555a61f0 "DROP TABLE public.event;\n", copyStmt =3D 0x= 0, dependencies =3D 0x0, nDeps =3D 0, dataDumper =3D 0x0, dataDumperArg = =3D 0x0, formatData =3D 0x555555584d40, 
  dataL= ength =3D 0, reqs =3D 1, created =3D false, pending_prev =3D 0x0, pendin= g_next =3D 0x0, depCount =3D 0, revDeps =3D 0x0, nRevDeps =3D 0, lockDep= s =3D 0x0, nLockDeps =3D 0}
(gdb) p te->dependencies[1]=
$21 =3D 3326
(gdb) p *AH->tocsByDumpId[3= 326]
$22 =3D {prev =3D 0x5555555a6650, next =3D 0x5555555a= 6910, catalogId =3D {tableoid =3D 0, oid =3D 0}, dumpId =3D 3326, sectio= n =3D SECTION_NONE, hadDumper =3D false, 
  tag = =3D 0x5555555854c0 "TABLE event", namespace =3D 0x555555585670 "public",= tablespace =3D 0x0, tableam =3D 0x0, owner =3D 0x555555585730 "euler", = desc =3D 0x555555585580 "ACL", 
  defn =3D 0x555= 5555a6890 "GRANT SELECT ON TABLE public.event TO role1;\nGRANT SELECT,IN= SERT,DELETE,UPDATE ON TABLE public.event TO role2;\n", dropStmt =3D 0x0,= copyStmt =3D 0x0, 
  dependencies =3D 0x5555555= a6050, nDeps =3D 1, dataDumper =3D 0x0, dataDumperArg =3D 0x0, formatDat= a =3D 0x555555585400, dataLength =3D 0, reqs =3D 1, created =3D false, p= ending_prev =3D 0x0, 
  pending_next =3D 0x0, de= pCount =3D 0, revDeps =3D 0x0, nRevDeps =3D 0, lockDeps =3D 0x0, nLockDe= ps =3D 0}

It means that an ACL for columns = has 2 dependencies (nDeps =3D 2) the function
_tocEntryReq= uired() returns 0 (see code below).

 &= nbsp;      else if (ropt->schemaNames.head != =3D NULL ||
       &nbs= p;         ropt->schemaExclud= eNames.head !=3D NULL ||
     &nb= sp;           ropt->= ;selTypes)
        {
          =   /*
        =      * In a selective dump/restore, we want to resto= re these dependent
      &nb= sp;      * TOC entry types only if their parent= object is being restored.
     &= nbsp;       * Without selectivity options,= we let through everything in the
    =          * archive.  Note t= here may be such entries with no parent, eg
  &n= bsp;          * non-default= ACLs for built-in objects.
     =         *
  &= nbsp;          * This code = depends on the parent having been marked already,
 &n= bsp;           * which= should be the case; if it isn't, perhaps due to
 &nb= sp;           * SortTo= cFromFile rearrangement, skipping the dependent entry
&nbs= p;            * s= eems prudent anyway.
      &= nbsp;      *
   &n= bsp;         * Ideally we'd hand= le, eg, table CHECK constraints this way too.
  =            * But it's = hard to tell which of their dependencies is the one to
&nb= sp;            * = consult.
        &= nbsp;    */
     &= nbsp;      if (te->nDeps !=3D 1 || 
=
          &= nbsp;     TocIDRequired(AH, te->dependencies[0]) = =3D=3D 0)
        =         return 0;
 = ;       }

Hen= ce, ProcessArchiveRestoreOptions() sets te->reqs to 0 that avoids the= ACL
for columns to be restored.

<= div>te->reqs =3D _tocEntryRequired(te, curSection, AH);

The discussion [1] in the commit 0d4e6ed3085 does not ex= plain if the 'if' logic
covers all cases. It certainly doe= sn't for the OP case. The only (hackish)
suggestion I have= ATM is to detect ACL for columns and returns 1.

--9ed1c71c70f44cd599122875703d07a8--