Re: BUG #16655: pg_dump segfault when excluding postgis table

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Stephen Frost <sfrost(at)snowman(dot)net>
Cc: cam(dot)daniel(at)gmail(dot)com, pgsql-bugs(at)lists(dot)postgresql(dot)org
Subject: Re: BUG #16655: pg_dump segfault when excluding postgis table
Date: 2020-10-06 19:06:58
Message-ID: 1995213.1602011218@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

I wrote:
> So I think the basic problem here is that checkExtensionMembership and
> processExtensionTables are not on the same page. We can't have
> interesting = false for a table that any of the dobj.dump bits are set
> for.

After studying this further, I've concluded that there are two independent
issues here. I was wrong to imagine that we can't get to dumpTableSchema
for tables that we're not going to dump; the actual intent of the code is
that it's going to run through that code anyway but not emit the
ArchiveEntry unless the DUMP_COMPONENT_DEFINITION flag is set. (This
supports cases where we only want to dump comments, for example.) So
dumpTableSchema really needs to cope with cases where getTableAttrs
has not loaded constraint info. There are a lot of other loops in the
code that expect that checkexprs[] is populated when ncheck > 0, too.
Maybe none of them are reachable for a not-interesting table, but
I don't think that's the way to bet. I think the right answer is to
redefine ncheck as being zero if we haven't loaded checkexprs[], as
per 0001 below.

0001 is sufficient to get past the proposed test case in 0002. However,
I think processExtensionTables is also buggy here. It does need to set
the "interesting" flag to true if it decides we need to dump the table's
data, because we have to have the per-attribute data to support that,
and "interesting" might not ever get set true on the table otherwise.
But *it is not OK to set "interesting" to false just because we don't
decide to dump the data*. If it's been set true for some other reason,
that other reason didn't just vanish. So I think we also need 0003.

BTW, the reason we can't get rid of the "interesting" flag is that
we need to set it on parent tables of tables we need to dump,
even if we're not going to dump the parents themselves (hence their
dobj.dump is zero). Otherwise we won't collect the subsidiary data
we need to figure out which parts of the child's definition are
inherited.

regards, tom lane

Attachment Content-Type Size
0001-postpone-setting-ncheck.patch text/x-diff 2.9 KB
0002-regression-test-case.patch text/x-diff 2.5 KB
0003-fix-interesting.patch text/x-diff 657 bytes

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Tom Lane 2020-10-06 21:26:04 Re: BUG #16655: pg_dump segfault when excluding postgis table
Previous Message Tom Lane 2020-10-06 17:01:47 Re: BUG #16655: pg_dump segfault when excluding postgis table