Horton

Thread

Commits

  1. pg_dump: Remove unnecessary code for security labels on extensions.

  2. pg_restore: Fix security label handling with --no-publications/subscriptions.

  3. pg_dump: Fix dumping of security labels on subscriptions and event triggers.

  4. pg_restore: Fix comment handling with --no-policies.

  5. pg_restore: Fix comment handling with --no-publications / --no-subscriptions.

  6. Core support for "extensions", which are packages of SQL objects.

  1. pg_restore --no-policies should not restore policies' comment

    jian he <jian.universality@gmail.com> — 2025-06-27T04:08:35Z

    hi.

first looking at function dumpPolicy (pg_dump.c):

    appendPQExpBuffer(polprefix, "POLICY %s ON",
                      fmtId(polinfo->polname));
    tag = psprintf("%s %s", tbinfo->dobj.name, polinfo->dobj.name);
  ....
    if (polinfo->dobj.dump & DUMP_COMPONENT_COMMENT)
        dumpComment(fout, polprefix->data, qtabname,
                    tbinfo->dobj.namespace->dobj.name, tbinfo->rolname,


then looking at function dumpCommentExtended:

        appendPQExpBuffer(query, "COMMENT ON %s ", type);
        if (namespace && *namespace)
            appendPQExpBuffer(query, "%s.", fmtId(namespace));
        appendPQExpBuffer(query, "%s IS ", name);
        appendStringLiteralAH(query, comments->descr, fout);
        appendPQExpBufferStr(query, ";\n");

        appendPQExpBuffer(tag, "%s %s", type, name);
        /*
         * We mark comments as SECTION_NONE because they really belong in the
         * same section as their parent, whether that is pre-data or
         * post-data.
         */
        ArchiveEntry(fout, nilCatalogId, createDumpId(),
                     ARCHIVE_OPTS(.tag = tag->data,
                                  .namespace = namespace,
                                  .owner = owner,
                                  .description = "COMMENT",
                                  .section = SECTION_NONE,
                                  .createStmt = query->data,
                                  .deps = &dumpId,
                                  .nDeps = 1));

also looking at function ArchiveEntry in pg_backup_archiver.c

    newToc->tag = pg_strdup(opts->tag);


if pg_restore --no-policies is specified then we generally don't want
to restore policies' comments too.
To do that, we need
1. we checked that COMMENTS on policies, the TocEntry->tag begins with
"POLICY". which is true, see above code walk through.
2. We also need to make sure that no other dumpComment call results in a
COMMENT command whose TocEntry->tag also starts with "POLICY".
which is also true, per https://www.postgresql.org/docs/current/sql-comment.html
after "COMMENT ON", the next word is fixed, and "POLICY" only occurs once.


If this is what we want, we can do the same for
"--no-publications", "--no-subscriptions" too.

  2. Re: pg_restore --no-policies should not restore policies' comment

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2025-06-27T05:34:35Z

    
On 2025/06/27 13:08, jian he wrote:
> hi.
> 
> first looking at function dumpPolicy (pg_dump.c):
> 
>      appendPQExpBuffer(polprefix, "POLICY %s ON",
>                        fmtId(polinfo->polname));
>      tag = psprintf("%s %s", tbinfo->dobj.name, polinfo->dobj.name);
>    ....
>      if (polinfo->dobj.dump & DUMP_COMPONENT_COMMENT)
>          dumpComment(fout, polprefix->data, qtabname,
>                      tbinfo->dobj.namespace->dobj.name, tbinfo->rolname,
> 
> 
> then looking at function dumpCommentExtended:
> 
>          appendPQExpBuffer(query, "COMMENT ON %s ", type);
>          if (namespace && *namespace)
>              appendPQExpBuffer(query, "%s.", fmtId(namespace));
>          appendPQExpBuffer(query, "%s IS ", name);
>          appendStringLiteralAH(query, comments->descr, fout);
>          appendPQExpBufferStr(query, ";\n");
> 
>          appendPQExpBuffer(tag, "%s %s", type, name);
>          /*
>           * We mark comments as SECTION_NONE because they really belong in the
>           * same section as their parent, whether that is pre-data or
>           * post-data.
>           */
>          ArchiveEntry(fout, nilCatalogId, createDumpId(),
>                       ARCHIVE_OPTS(.tag = tag->data,
>                                    .namespace = namespace,
>                                    .owner = owner,
>                                    .description = "COMMENT",
>                                    .section = SECTION_NONE,
>                                    .createStmt = query->data,
>                                    .deps = &dumpId,
>                                    .nDeps = 1));
> 
> also looking at function ArchiveEntry in pg_backup_archiver.c
> 
>      newToc->tag = pg_strdup(opts->tag);
> 
> 
> if pg_restore --no-policies is specified then we generally don't want
> to restore policies' comments too.

Agreed. Otherwise, pg_restore --no-policies might skip creating
the policy object but still try to run a COMMENT command on it,
which would fail since the policy object doesn't exist.


> To do that, we need
> 1. we checked that COMMENTS on policies, the TocEntry->tag begins with
> "POLICY". which is true, see above code walk through.
> 2. We also need to make sure that no other dumpComment call results in a
> COMMENT command whose TocEntry->tag also starts with "POLICY".
> which is also true, per https://www.postgresql.org/docs/current/sql-comment.html
> after "COMMENT ON", the next word is fixed, and "POLICY" only occurs once.
> 
> 
> If this is what we want, we can do the same for
> "--no-publications", "--no-subscriptions" too.

Agreed.

Regards,

-- 
Fujii Masao
NTT DATA Japan Corporation

  3. Re: pg_restore --no-policies should not restore policies' comment

    jian he <jian.universality@gmail.com> — 2025-07-02T02:17:09Z

    On Fri, Jun 27, 2025 at 1:34 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
>
> > To do that, we need
> > 1. we checked that COMMENTS on policies, the TocEntry->tag begins with
> > "POLICY". which is true, see above code walk through.
> > 2. We also need to make sure that no other dumpComment call results in a
> > COMMENT command whose TocEntry->tag also starts with "POLICY".
> > which is also true, per https://www.postgresql.org/docs/current/sql-comment.html
> > after "COMMENT ON", the next word is fixed, and "POLICY" only occurs once.
> >
> >
> > If this is what we want, we can do the same for
> > "--no-publications", "--no-subscriptions" too.
>
> Agreed.
>

hi.

I’ve tested the pg_restore options --no-policies, --no-publications, and
--no-subscriptions locally.
However, I haven’t tested --no-security-labels option, so no changes were
made for it.  Testing --no-security-labels appears to need more setup, which
didn’t seem trivial.

writing Perl tests is not easier for me, I didn’t add those either.
(seems in master, we didn't have --no-publications, --no-subscriptions
tests too)

  4. Re: pg_restore --no-policies should not restore policies' comment

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2025-07-02T09:18:00Z

    
On 2025/07/02 11:17, jian he wrote:
> On Fri, Jun 27, 2025 at 1:34 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
>>
>>> To do that, we need
>>> 1. we checked that COMMENTS on policies, the TocEntry->tag begins with
>>> "POLICY". which is true, see above code walk through.
>>> 2. We also need to make sure that no other dumpComment call results in a
>>> COMMENT command whose TocEntry->tag also starts with "POLICY".
>>> which is also true, per https://www.postgresql.org/docs/current/sql-comment.html
>>> after "COMMENT ON", the next word is fixed, and "POLICY" only occurs once.
>>>
>>>
>>> If this is what we want, we can do the same for
>>> "--no-publications", "--no-subscriptions" too.
>>
>> Agreed.
>>
> 
> hi.
> 
> I’ve tested the pg_restore options --no-policies, --no-publications, and
> --no-subscriptions locally.

Thanks for updating the patch! Could you add it to the next CommitFest
so we don't forget about it?


> However, I haven’t tested --no-security-labels option, so no changes were
> made for it.  Testing --no-security-labels appears to need more setup, which
> didn’t seem trivial.

You're checking whether pg_restore --no-publications --no-subscriptions correctly
skips security labels for publications and subscriptions, and if not,
you'll prepare a patch. Right? I'm not sure how common it is to define security
labels on publications or subscriptions, but if the behavior is unexpected (i.e.,
the security labels are not skipped in that case), it's worth fixing.
It would probably be better to handle that in a separate patch from the one
for comments.

To set up a security label for testing, you can use the
src/test/modules/dummy_seclabel module.

Regards,

-- 
Fujii Masao
NTT DATA Japan Corporation

  5. Re: pg_restore --no-policies should not restore policies' comment

    jian he <jian.universality@gmail.com> — 2025-07-03T14:31:13Z

    On Wed, Jul 2, 2025 at 5:18 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
>
> > hi.
> >
> > I’ve tested the pg_restore options --no-policies, --no-publications, and
> > --no-subscriptions locally.
>
> Thanks for updating the patch! Could you add it to the next CommitFest
> so we don't forget about it?
>
sure.

>
> > However, I haven’t tested --no-security-labels option, so no changes were
> > made for it.  Testing --no-security-labels appears to need more setup, which
> > didn’t seem trivial.
>
> You're checking whether pg_restore --no-publications --no-subscriptions correctly
> skips security labels for publications and subscriptions, and if not,
> you'll prepare a patch. Right? I'm not sure how common it is to define security
> labels on publications or subscriptions, but if the behavior is unexpected (i.e.,
> the security labels are not skipped in that case), it's worth fixing.
> It would probably be better to handle that in a separate patch from the one
> for comments.
>
> To set up a security label for testing, you can use the
> src/test/modules/dummy_seclabel module.
>

Thanks!
Using dummy_seclabel helped me test the pg_restore --no-security-labels
behavior.  All the attached patches have now been tested locally. I’ll need help
writing the Perl tests....

I found out another problem while playing with pg_restore --no-security-labels:
pg_dump does not dump security labels on global objects like
subscriptions or roles.

summary of attached patch:
01: make pg_restore not restore comments if comments associated
objects are excluded.
02: make pg_dump dump security label for shared database objects, like
subscription, roles.
03: make pg_restore not restore security labels if the associated
object is excluded.

  6. Re: pg_restore --no-policies should not restore policies' comment

    Fujii Masao <masao.fujii@gmail.com> — 2025-08-26T11:43:45Z

    On Thu, Jul 3, 2025 at 11:32 PM jian he <jian.universality@gmail.com> wrote:
>
> On Wed, Jul 2, 2025 at 5:18 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
> >
> > > hi.
> > >
> > > I’ve tested the pg_restore options --no-policies, --no-publications, and
> > > --no-subscriptions locally.
> >
> > Thanks for updating the patch! Could you add it to the next CommitFest
> > so we don't forget about it?
> >
> sure.
>
> >
> > > However, I haven’t tested --no-security-labels option, so no changes were
> > > made for it.  Testing --no-security-labels appears to need more setup, which
> > > didn’t seem trivial.
> >
> > You're checking whether pg_restore --no-publications --no-subscriptions correctly
> > skips security labels for publications and subscriptions, and if not,
> > you'll prepare a patch. Right? I'm not sure how common it is to define security
> > labels on publications or subscriptions, but if the behavior is unexpected (i.e.,
> > the security labels are not skipped in that case), it's worth fixing.
> > It would probably be better to handle that in a separate patch from the one
> > for comments.
> >
> > To set up a security label for testing, you can use the
> > src/test/modules/dummy_seclabel module.
> >
>
> Thanks!
> Using dummy_seclabel helped me test the pg_restore --no-security-labels
> behavior.  All the attached patches have now been tested locally. I’ll need help
> writing the Perl tests....
>
> I found out another problem while playing with pg_restore --no-security-labels:
> pg_dump does not dump security labels on global objects like
> subscriptions or roles.
>
> summary of attached patch:

Thanks for the patches!

> 01: make pg_restore not restore comments if comments associated
> objects are excluded.

> TODO: need perl tests

How about adding tests for pg_restore --no-policies in 002_pg_dump.pl,
as in the attached patch?

Since --no-publications and --no-subscriptions have been around for a long time,
while --no-policies was added in v18, I wonder if it makes sense to first fix
the publications and subscriptions cases (and add tests for them) and back-patch
to all supported versions. Then we can handle the policies case and
back-patch it
only to v18. Does that sound reasonable?


> 02: make pg_dump dump security label for shared database objects, like
> subscription, roles.
> 03: make pg_restore not restore security labels if the associated
> object is excluded.

Will review them later.

Regards,

-- 
Fujii Masao

  7. Re: pg_restore --no-policies should not restore policies' comment

    jian he <jian.universality@gmail.com> — 2025-08-27T06:17:41Z

    On Tue, Aug 26, 2025 at 7:43 PM Fujii Masao <masao.fujii@gmail.com> wrote:
>
> >
> > summary of attached patch:
>
> Thanks for the patches!
>
> > 01: make pg_restore not restore comments if comments associated
> > objects are excluded.
>
> > TODO: need perl tests
>
> How about adding tests for pg_restore --no-policies in 002_pg_dump.pl,
> as in the attached patch?
>

it works fine on my local linux machine.

> Since --no-publications and --no-subscriptions have been around for a long time,
> while --no-policies was added in v18, I wonder if it makes sense to first fix
> the publications and subscriptions cases (and add tests for them) and back-patch
> to all supported versions. Then we can handle the policies case and
> back-patch it
> only to v18. Does that sound reasonable?
>
works for me.

  8. Re: pg_restore --no-policies should not restore policies' comment

    Fujii Masao <masao.fujii@gmail.com> — 2025-09-03T11:50:09Z

    On Wed, Aug 27, 2025 at 3:18 PM jian he <jian.universality@gmail.com> wrote:
> > Since --no-publications and --no-subscriptions have been around for a long time,
> > while --no-policies was added in v18, I wonder if it makes sense to first fix
> > the publications and subscriptions cases (and add tests for them) and back-patch
> > to all supported versions. Then we can handle the policies case and
> > back-patch it
> > only to v18. Does that sound reasonable?
> >
> works for me.

So I've split your v2-0001 patch into two patches:

* v4-0001 handles comments on publications and subscriptions when
   --no-publications / --no-subscriptions are specified. This will be
   backpatched to all supported versions.

* v4-0002 handles comments on policies when --no-policies is specified.
   This will be backpatched to v18, where --no-policies was added.

Both v4-0001 and v4-0002 are based on your patch, but I added
regression tests for them.


> > > 02: make pg_dump dump security label for shared database objects, like
> > > subscription, roles.

As I understand it, shared objects like roles are handled by pg_dumpall,
which already dumps their security labels via pg_shseclabel.
Subscriptions are an exception: pg_dump dumps them (and should dump
their security labels), but those labels are stored in pg_shseclabel,
which pg_dump doesn't query.

To fix this, making pg_dump query also pg_shseclabel when dumping
subscriptions would work. But your approach, having pg_dump query
pg_seclabels (covering both pg_seclabel and pg_shseclabel),
is simpler and sufficient. So I like your approach for now.

I also noticed pg_dump didn't dump security labels on event triggers,
so I extended your patch as v4-0003 to handle those as well.


> > > 03: make pg_restore not restore security labels if the associated
> > > object is excluded.

This patch looks good. I only applied minor cosmetic changes and
attached it as v4-0004.

Regards,

-- 
Fujii Masao

  9. Re: pg_restore --no-policies should not restore policies' comment

    jian he <jian.universality@gmail.com> — 2025-09-04T09:00:02Z

    On Wed, Sep 3, 2025 at 7:50 PM Fujii Masao <masao.fujii@gmail.com> wrote:
>
> > > > 02: make pg_dump dump security label for shared database objects, like
> > > > subscription, roles.
>
> As I understand it, shared objects like roles are handled by pg_dumpall,
> which already dumps their security labels via pg_shseclabel.
> Subscriptions are an exception: pg_dump dumps them (and should dump
> their security labels), but those labels are stored in pg_shseclabel,
> which pg_dump doesn't query.
>
> To fix this, making pg_dump query also pg_shseclabel when dumping
> subscriptions would work. But your approach, having pg_dump query
> pg_seclabels (covering both pg_seclabel and pg_shseclabel),
> is simpler and sufficient. So I like your approach for now.
>
> I also noticed pg_dump didn't dump security labels on event triggers,
> so I extended your patch as v4-0003 to handle those as well.
>
>

in _tocEntryRestorePass
if we do

    if ((strcmp(te->desc, "COMMENT") == 0 ||
         strcmp(te->desc, "SECURITY LABEL") == 0) &&
        strncmp(te->tag, "EVENT TRIGGER ", 14) == 0)
        return RESTORE_PASS_POST_ACL;

then RestorePass related comments also need to be adjusted for security label?

typedef enum
{
    RESTORE_PASS_MAIN = 0,        /* Main pass (most TOC item types) */
    RESTORE_PASS_ACL,            /* ACL item types */
    RESTORE_PASS_POST_ACL,        /* Event trigger and matview refresh items */

#define RESTORE_PASS_LAST RESTORE_PASS_POST_ACL
} RestorePass;

we do not support security label on extension, see SecLabelSupportsObjectType.
below the dumpExtension function code should be removed?

   /* Dump Extension Comments and Security Labels */
    if (extinfo->dobj.dump & DUMP_COMPONENT_SECLABEL)
        dumpSecLabel(fout, "EXTENSION", qextname,
                     NULL, "",
                     extinfo->dobj.catId, 0, extinfo->dobj.dumpId);

  10. Re: pg_restore --no-policies should not restore policies' comment

    Fujii Masao <masao.fujii@gmail.com> — 2025-09-09T04:00:15Z

    On Thu, Sep 4, 2025 at 6:00 PM jian he <jian.universality@gmail.com> wrote:
> in _tocEntryRestorePass
> if we do
>
>     if ((strcmp(te->desc, "COMMENT") == 0 ||
>          strcmp(te->desc, "SECURITY LABEL") == 0) &&
>         strncmp(te->tag, "EVENT TRIGGER ", 14) == 0)
>         return RESTORE_PASS_POST_ACL;
>
> then RestorePass related comments also need to be adjusted for security label?

Could you clarify which comments should be updated for SECURITY LABEL?
I don't see any references to COMMENT in RestorePass, so it doesn't look
like any updates are needed there for SECURITY LABEL either. But maybe
I'm missing something...


> we do not support security label on extension, see SecLabelSupportsObjectType.
> below the dumpExtension function code should be removed?
>
>    /* Dump Extension Comments and Security Labels */
>     if (extinfo->dobj.dump & DUMP_COMPONENT_SECLABEL)
>         dumpSecLabel(fout, "EXTENSION", qextname,
>                      NULL, "",
>                      extinfo->dobj.catId, 0, extinfo->dobj.dumpId);

Good catch! This code was introduced in commit d9572c4e3b4,
which added core support for extensions. Since security labels on
extensions are not supported, I agree that the code should be
removed in master.

Regards,

-- 
Fujii Masao

  11. Re: pg_restore --no-policies should not restore policies' comment

    jian he <jian.universality@gmail.com> — 2025-09-09T05:49:06Z

    On Tue, Sep 9, 2025 at 12:00 PM Fujii Masao <masao.fujii@gmail.com> wrote:
>
> On Thu, Sep 4, 2025 at 6:00 PM jian he <jian.universality@gmail.com> wrote:
> > in _tocEntryRestorePass
> > if we do
> >
> >     if ((strcmp(te->desc, "COMMENT") == 0 ||
> >          strcmp(te->desc, "SECURITY LABEL") == 0) &&
> >         strncmp(te->tag, "EVENT TRIGGER ", 14) == 0)
> >         return RESTORE_PASS_POST_ACL;
> >
> > then RestorePass related comments also need to be adjusted for security label?
>
> Could you clarify which comments should be updated for SECURITY LABEL?
> I don't see any references to COMMENT in RestorePass, so it doesn't look
> like any updates are needed there for SECURITY LABEL either. But maybe
> I'm missing something...
>

typedef enum
{
    RESTORE_PASS_MAIN = 0,        /* Main pass (most TOC item types) */
    RESTORE_PASS_ACL,            /* ACL item types */
    RESTORE_PASS_POST_ACL,        /* Event trigger and matview refresh items */

#define RESTORE_PASS_LAST RESTORE_PASS_POST_ACL
} RestorePass;

I thought we needed to change the comments:
" /* Event trigger and matview refresh items */".

looking at the code again, we don't need to do that.
Overall, all the patches look good to me.
(I didn't test back branch related tests though)

  12. Re: pg_restore --no-policies should not restore policies' comment

    Fujii Masao <masao.fujii@gmail.com> — 2025-09-09T07:56:15Z

    On Tue, Sep 9, 2025 at 2:50 PM jian he <jian.universality@gmail.com> wrote:
>
> On Tue, Sep 9, 2025 at 12:00 PM Fujii Masao <masao.fujii@gmail.com> wrote:
> >
> > On Thu, Sep 4, 2025 at 6:00 PM jian he <jian.universality@gmail.com> wrote:
> > > in _tocEntryRestorePass
> > > if we do
> > >
> > >     if ((strcmp(te->desc, "COMMENT") == 0 ||
> > >          strcmp(te->desc, "SECURITY LABEL") == 0) &&
> > >         strncmp(te->tag, "EVENT TRIGGER ", 14) == 0)
> > >         return RESTORE_PASS_POST_ACL;
> > >
> > > then RestorePass related comments also need to be adjusted for security label?
> >
> > Could you clarify which comments should be updated for SECURITY LABEL?
> > I don't see any references to COMMENT in RestorePass, so it doesn't look
> > like any updates are needed there for SECURITY LABEL either. But maybe
> > I'm missing something...
> >
>
> typedef enum
> {
>     RESTORE_PASS_MAIN = 0,        /* Main pass (most TOC item types) */
>     RESTORE_PASS_ACL,            /* ACL item types */
>     RESTORE_PASS_POST_ACL,        /* Event trigger and matview refresh items */
>
> #define RESTORE_PASS_LAST RESTORE_PASS_POST_ACL
> } RestorePass;
>
> I thought we needed to change the comments:
> " /* Event trigger and matview refresh items */".
>
> looking at the code again, we don't need to do that.
> Overall, all the patches look good to me.
> (I didn't test back branch related tests though)

Thanks for the review! Unless there are any objections,
I’ll run the tests again and then push the patches.

Regards,

-- 
Fujii Masao

  13. Re: pg_restore --no-policies should not restore policies' comment

    Fujii Masao <masao.fujii@gmail.com> — 2025-09-16T08:04:18Z

    On Tue, Sep 9, 2025 at 4:56 PM Fujii Masao <masao.fujii@gmail.com> wrote:
> Thanks for the review! Unless there are any objections,
> I’ll run the tests again and then push the patches.

I've pushed patches 0001, 0002, and 0003. I'll push 0004 later.

Buildfarm member petalura reported a failure at [1] after 0001 was backpatched
to v13. However, the server logs show a "double free or corruption" error
occurring as the backend was exiting. Since that code was not touched by 0001,
for now I don't think this failure is related to the patch...
But am I missing something?

----------------------------------------
2025-09-16 05:47:21.231 CEST [226419][client backend][10/2179:0] LOG:
statement: RESET client_min_messages;
2025-09-16 05:47:21.231 CEST [226419][client backend][10/2180:0] LOG:
statement: RESET search_path;
2025-09-16 05:47:21.244 CEST [226419][client backend][:0] LOG:
disconnection: session time: 0:00:35.551 user=bf database=regression
host=[local]
double free or corruption (!prev)
<snip>
2025-09-16 05:47:21.550 CEST [10340][postmaster][:0] LOG:  server
process (PID 226419) was terminated by signal 6: Aborted
2025-09-16 05:47:21.550 CEST [10340][postmaster][:0] LOG:  terminating
any other active server processes
----------------------------------------

Regards,

[1] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=petalura&dt=2025-09-16%2003%3A29%3A05

-- 
Fujii Masao

  14. Re: pg_restore --no-policies should not restore policies' comment

    Fujii Masao <masao.fujii@gmail.com> — 2025-09-18T03:03:24Z

    On Tue, Sep 16, 2025 at 5:04 PM Fujii Masao <masao.fujii@gmail.com> wrote:
>
> On Tue, Sep 9, 2025 at 4:56 PM Fujii Masao <masao.fujii@gmail.com> wrote:
> > Thanks for the review! Unless there are any objections,
> > I’ll run the tests again and then push the patches.
>
> I've pushed patches 0001, 0002, and 0003. I'll push 0004 later.

I've pushed 0004, thanks!

> Buildfarm member petalura reported a failure at [1] after 0001 was backpatched
> to v13. However, the server logs show a "double free or corruption" error
> occurring as the backend was exiting. Since that code was not touched by 0001,
> for now I don't think this failure is related to the patch...
> But am I missing something?

After pushing 0004, petalura reported another failure with a "double
free or corruption" error:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=petalura&dt=2025-09-18%2002%3A14%3A32

Buildfarm member phycodurus also reported similar failures:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=phycodurus&dt=2025-09-16%2011%3A09%3A07
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=phycodurus&dt=2025-09-16%2016%3A44%3A37

I'll dig deeper into this issue.


As for the unnecessary code for security labels on extensions
you mentioned earlier, I've created a patch to remove it. Patch attached.

Regards,

-- 
Fujii Masao

  15. Re: pg_restore --no-policies should not restore policies' comment

    jian he <jian.universality@gmail.com> — 2025-10-16T14:22:52Z

    On Thu, Sep 18, 2025 at 11:03 AM Fujii Masao <masao.fujii@gmail.com> wrote:
>
> On Tue, Sep 16, 2025 at 5:04 PM Fujii Masao <masao.fujii@gmail.com> wrote:
> >
> > On Tue, Sep 9, 2025 at 4:56 PM Fujii Masao <masao.fujii@gmail.com> wrote:
> > > Thanks for the review! Unless there are any objections,
> > > I’ll run the tests again and then push the patches.
> >
> > I've pushed patches 0001, 0002, and 0003. I'll push 0004 later.
>
> I've pushed 0004, thanks!
>
> > Buildfarm member petalura reported a failure at [1] after 0001 was backpatched
> > to v13. However, the server logs show a "double free or corruption" error
> > occurring as the backend was exiting. Since that code was not touched by 0001,
> > for now I don't think this failure is related to the patch...
> > But am I missing something?
>
> After pushing 0004, petalura reported another failure with a "double
> free or corruption" error:
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=petalura&dt=2025-09-18%2002%3A14%3A32
>
> Buildfarm member phycodurus also reported similar failures:
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=phycodurus&dt=2025-09-16%2011%3A09%3A07
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=phycodurus&dt=2025-09-16%2016%3A44%3A37
>
> I'll dig deeper into this issue.
>

It appears that this issue no longer exists, per https://bfbot.cputube.org

> As for the unnecessary code for security labels on extensions
> you mentioned earlier, I've created a patch to remove it. Patch attached.

looks good to me.

  16. Re: pg_restore --no-policies should not restore policies' comment

    Alexander Law <exclusion@gmail.com> — 2025-10-17T05:00:00Z

    Hello Jian,

16.10.2025 17:22, jian he пишет:
>>> Buildfarm member petalura reported a failure at [1] after 0001 was backpatched
>>> to v13. However, the server logs show a "double free or corruption" error
>>> occurring as the backend was exiting. Since that code was not touched by 0001,
>>> for now I don't think this failure is related to the patch...
>>> But am I missing something?
>> After pushing 0004, petalura reported another failure with a "double
>> free or corruption" error:
>> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=petalura&dt=2025-09-18%2002%3A14%3A32
>>
>> Buildfarm member phycodurus also reported similar failures:
>> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=phycodurus&dt=2025-09-16%2011%3A09%3A07
>> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=phycodurus&dt=2025-09-16%2016%3A44%3A37
>>
>> I'll dig deeper into this issue.
>>
> It appears that this issue no longer exists, perhttps://bfbot.cputube.org

Unfortunately, buildfarm animals (especially phycodurus) are still
failing due to "double free or corruption (!prev)":
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=phycodurus&dt=2025-10-16%2019%3A07%3A52

I can try to reproduce this locally if it can be helpful.

Best regards,
Alexander

  17. Re: pg_restore --no-policies should not restore policies' comment

    Fujii Masao <masao.fujii@gmail.com> — 2025-10-17T10:23:22Z

    On Thu, Oct 16, 2025 at 11:23 PM jian he <jian.universality@gmail.com> wrote:
> > As for the unnecessary code for security labels on extensions
> > you mentioned earlier, I've created a patch to remove it. Patch attached.
>
> looks good to me.

Thanks for the review! Unless there are any objections, I'll commit the patch.

By the way, while reading the documentation about security labels and
extensions, I noticed this section:

https://www.postgresql.org/docs/devel/extend-extensions.html
> PostgreSQL does not currently support extension scripts issuing CREATE POLICY
> or SECURITY LABEL statements. These are expected to be set after
> the extension has been created. All RLS policies and security labels on
> extension objects will be included in dumps created by pg_dump.

I'm not sure the last sentence is accurate - in my quick test, a security label
on the pgstattuple function wasn't included in the dump. If that's correct,
should we update this part of the documentation, as a separate patch?

------------------------
$ psql
=# CREATE EXTENSION dummy_seclabel ;
=# CREATE EXTENSION pgstattuple ;
=# SECURITY LABEL ON FUNCTION pgstattuple(regclass) IS 'classified';
=# \q

$ pg_dump | grep -i "security label"
COMMENT ON EXTENSION dummy_seclabel IS 'Test code for SECURITY LABEL feature';
------------------------

In the above example, SECURITY LABEL command for pgstattuple function
was not included in the dump.

Regards,

-- 
Fujii Masao

  18. Re: pg_restore --no-policies should not restore policies' comment

    Fujii Masao <masao.fujii@gmail.com> — 2025-10-17T10:32:35Z

    On Fri, Oct 17, 2025 at 2:00 PM Alexander Lakhin <exclusion@gmail.com> wrote:
> Unfortunately, buildfarm animals (especially phycodurus) are still
> failing due to "double free or corruption (!prev)":
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=phycodurus&dt=2025-10-16%2019%3A07%3A52

Yeah... A new thread [1] has been started about this issue. I'll post
any findings there.

[1]
https://postgr.es/m/aPAl27_urHSODwRN@paquier.xyz

> I can try to reproduce this locally if it can be helpful.

Yes, that's really helpful.

Regards,

-- 
Fujii Masao

  19. Re: pg_restore --no-policies should not restore policies' comment

    Fujii Masao <masao.fujii@gmail.com> — 2025-10-20T02:47:20Z

    On Fri, Oct 17, 2025 at 7:23 PM Fujii Masao <masao.fujii@gmail.com> wrote:
>
> On Thu, Oct 16, 2025 at 11:23 PM jian he <jian.universality@gmail.com> wrote:
> > > As for the unnecessary code for security labels on extensions
> > > you mentioned earlier, I've created a patch to remove it. Patch attached.
> >
> > looks good to me.
>
> Thanks for the review! Unless there are any objections, I'll commit the patch.

I've pushed the patch. Thanks!

Regards,

-- 
Fujii Masao