Обсуждение: Truncation of mapped catalogs (whether local or shared) leads to server crash

Hi everyone,

I've noticed that truncating mapped catalogs causes the server to
crash due to an assertion failure. Here are the details:

Executing below commands:

-- set allow_system_table_mods TO on;
-- truncate table pg_type;

Results into a server crash with below backtrace:

...
#2  0x000055736767537d in ExceptionalCondition
(conditionName=0x5573678c5760 "relation->rd_rel->relkind ==
RELKIND_INDEX", fileName=0x5573678c4b28 "relcache.c",
    lineNumber=3896) at assert.c:66
#3  0x0000557367664e31 in RelationSetNewRelfilenumber
(relation=0x7f68240f1d58, persistence=112 'p') at relcache.c:3896
#4  0x000055736715b952 in ExecuteTruncateGuts
(explicit_rels=0x55736989e5b0, relids=0x55736989e600,
relids_logged=0x0, behavior=DROP_RESTRICT, restart_seqs=false,
    run_as_table_owner=false) at tablecmds.c:2146
#5  0x000055736715affa in ExecuteTruncate (stmt=0x55736989f950) at
tablecmds.c:1877
#6  0x0000557367493693 in standard_ProcessUtility
(pstmt=0x55736989fa00, queryString=0x55736989eed0 "truncate table
pg_type;", readOnlyTree=false,
    context=PROCESS_UTILITY_TOPLEVEL, params=0x0, queryEnv=0x0,
dest=0x5573698a0330, qc=0x7ffe19367ac0) at utility.c:728

As seen from the backtrace above, the assertion failure occurs in
'RelationSetNewRelfilenumber()' at:

if (RelationIsMapped(relation))
{
    /* This case is only supported for indexes */
    Assert(relation->rd_rel->relkind == RELKIND_INDEX);
}

I would like to know why we are only expecting index tables here and
not the regular catalog tables. For instance, pg_type is a mapped
relation but not of index type, leading to the failure in this case.
Should we consider changing this Assert condition from RELKIND_INDEX
to (RELKIND_INDEX || RELKIND_RELATION)?

Additionally, is it advisable to restrict truncation of the pg_class
table? It's like a kind of circular dependency in case of pg_class
which is not applicable in case of other catalog tables.

--
With Regards,
Ashutosh Sharma.

On Tue, Jun 18, 2024 at 8:10 AM Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
> I've noticed that truncating mapped catalogs causes the server to
> crash due to an assertion failure. Here are the details:
>
> Executing below commands:
>
> -- set allow_system_table_mods TO on;
> -- truncate table pg_type;

If the operation isn't allowed without turning on
allow_system_table_mods, that means that doing it is probably a bad
idea and will probably break stuff, as happened here.

--
Robert Haas
EDB: http://www.enterprisedb.com

Robert Haas <robertmhaas@gmail.com> writes:
> On Tue, Jun 18, 2024 at 8:10 AM Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
>> Executing below commands:
>> -- set allow_system_table_mods TO on;
>> -- truncate table pg_type;

> If the operation isn't allowed without turning on
> allow_system_table_mods, that means that doing it is probably a bad
> idea and will probably break stuff, as happened here.

Nothing good can come of truncating *any* core system catalog --- what
do you think you'll still be able to do afterwards?

I think the assertion you noticed is there because the code path gets
traversed during REINDEX, which is an operation we do support on
system catalogs.  I have zero interest in making truncate work
on them.

            regards, tom lane

Hi,

On Tue, Jun 18, 2024 at 7:50 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Robert Haas <robertmhaas@gmail.com> writes:
> > On Tue, Jun 18, 2024 at 8:10 AM Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
> >> Executing below commands:
> >> -- set allow_system_table_mods TO on;
> >> -- truncate table pg_type;
>
> > If the operation isn't allowed without turning on
> > allow_system_table_mods, that means that doing it is probably a bad
> > idea and will probably break stuff, as happened here.
>
> Nothing good can come of truncating *any* core system catalog --- what
> do you think you'll still be able to do afterwards?
>
> I think the assertion you noticed is there because the code path gets
> traversed during REINDEX, which is an operation we do support on
> system catalogs.  I have zero interest in making truncate work
> on them.
>

I agree with you on that point. How about considering a complete
restriction instead?

--
With Regards,
Ashutosh Sharma.

Hi,

On 2024-06-18 19:58:26 +0530, Ashutosh Sharma wrote:
> On Tue, Jun 18, 2024 at 7:50 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >
> > Robert Haas <robertmhaas@gmail.com> writes:
> > > On Tue, Jun 18, 2024 at 8:10 AM Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
> > >> Executing below commands:
> > >> -- set allow_system_table_mods TO on;
> > >> -- truncate table pg_type;
> >
> > > If the operation isn't allowed without turning on
> > > allow_system_table_mods, that means that doing it is probably a bad
> > > idea and will probably break stuff, as happened here.
> >
> > Nothing good can come of truncating *any* core system catalog --- what
> > do you think you'll still be able to do afterwards?
> >
> > I think the assertion you noticed is there because the code path gets
> > traversed during REINDEX, which is an operation we do support on
> > system catalogs.  I have zero interest in making truncate work
> > on them.
> >
> 
> I agree with you on that point. How about considering a complete
> restriction instead?

What's the point?  There are occasional cases where doing something dangerous
is useful, for debugging or corruption recovery. If we flat out prohibit this
we'll just need a allow_system_table_mods_but_for_real option.

Greetings,

Andres Freund

Ashutosh Sharma <ashu.coek88@gmail.com> writes:
> On Tue, Jun 18, 2024 at 7:50 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I think the assertion you noticed is there because the code path gets
>> traversed during REINDEX, which is an operation we do support on
>> system catalogs.  I have zero interest in making truncate work
>> on them.

> I agree with you on that point. How about considering a complete
> restriction instead?

You already broke the safety seal by enabling allow_system_table_mods.
Perhaps the documentation of that is not scary enough?

        Allows modification of the structure of system tables as well as
        certain other risky actions on system tables.  This is otherwise not
        allowed even for superusers.  Ill-advised use of this setting can
        cause irretrievable data loss or seriously corrupt the database
        system.

            regards, tom lane

Hi Robert, Andres, Tom,

Thank you for sharing your thoughts.

On Tue, Jun 18, 2024 at 8:02 PM Andres Freund <andres@anarazel.de> wrote:
>
> Hi,
>
> On 2024-06-18 19:58:26 +0530, Ashutosh Sharma wrote:
> > On Tue, Jun 18, 2024 at 7:50 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > >
> > > Robert Haas <robertmhaas@gmail.com> writes:
> > > > On Tue, Jun 18, 2024 at 8:10 AM Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
> > > >> Executing below commands:
> > > >> -- set allow_system_table_mods TO on;
> > > >> -- truncate table pg_type;
> > >
> > > > If the operation isn't allowed without turning on
> > > > allow_system_table_mods, that means that doing it is probably a bad
> > > > idea and will probably break stuff, as happened here.
> > >
> > > Nothing good can come of truncating *any* core system catalog --- what
> > > do you think you'll still be able to do afterwards?
> > >
> > > I think the assertion you noticed is there because the code path gets
> > > traversed during REINDEX, which is an operation we do support on
> > > system catalogs.  I have zero interest in making truncate work
> > > on them.
> > >
> >
> > I agree with you on that point. How about considering a complete
> > restriction instead?
>
> What's the point?  There are occasional cases where doing something dangerous
> is useful, for debugging or corruption recovery. If we flat out prohibit this
> we'll just need a allow_system_table_mods_but_for_real option.
>

This is specifically about truncation of system catalogs, and does not
refer to any other DML operations on system catalogs, which I see are
necessary for many extensions that directly update catalogs like
pg_proc and others. Additionally, according to the comments in
truncate_check_rel(), we permit truncation of the pg_largeobject
catalog specifically during pg_upgrade. So, afaiu truncation of any
catalogs other than this can be restricted.

--
With Regards,
Ashutosh Sharma.

Hi,

On Tue, Jun 18, 2024 at 8:25 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Ashutosh Sharma <ashu.coek88@gmail.com> writes:
> > On Tue, Jun 18, 2024 at 7:50 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >> I think the assertion you noticed is there because the code path gets
> >> traversed during REINDEX, which is an operation we do support on
> >> system catalogs.  I have zero interest in making truncate work
> >> on them.
>
> > I agree with you on that point. How about considering a complete
> > restriction instead?
>
> You already broke the safety seal by enabling allow_system_table_mods.
> Perhaps the documentation of that is not scary enough?
>
>         Allows modification of the structure of system tables as well as
>         certain other risky actions on system tables.  This is otherwise not
>         allowed even for superusers.  Ill-advised use of this setting can
>         cause irretrievable data loss or seriously corrupt the database
>         system.
>

I was actually referring to just the truncation part here, not any DML
operations, as I've observed their usage in certain extensions.
However, truncation is just used for pg_largeobject and that too only
during pg_upgrade, so for other catalogs truncation can be avoided.
But that is just my perspective; if it's not essential, we can
possibly stop this discussion here.

Thank you to everyone for sharing your valuable insights.

--
With Regards,
Ashutosh Sharma.