Thread: BUG #15343: Segmentation fault using pg_dump with --exclude-table iftable contains identity column

The following bug has been logged on the website:

Bug reference:      15343
Logged by:          Andy Abelisto
Email address:      abelisto@gmail.com
PostgreSQL version: 10.5
Operating system:   Linux Mint 19 Tara
Description:

To reproduce:
$ createdb foo
$ psql foo -c "create table bar(i int generated by default as identity
primary key);"
$ pg_dump foo --exclude-table=bar
Segmentation fault (core dumped)

> To reproduce:
> $ createdb foo
> $ psql foo -c "create table bar(i int generated by default as identity
> primary key);"
> $ pg_dump foo --exclude-table=bar
> Segmentation fault (core dumped)

This crashes during dumpSequence() when the owning_tab's attnames are
accessed. This array is NULL since we didn't populate it in
getTableAttrs due to the table being !interesting.

I think the fix depends exactly on what we want the behaviour to be
here.  Should dumpSequence() still dump out a CREATE SEQUENCE
statement for this, or should we just ignore the sequence?  I imagine
we should just ignore identity sequences when we're not going to dump
the table that owns them. It looks like that would require a small
modification in getOwnedSeqs().  Otherwise we'd need something like
the attached to still dump out the CREATE SEQUENCE.

-- 
 David Rowley                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

On 2018-Aug-21, David Rowley wrote:

> > To reproduce:
> > $ createdb foo
> > $ psql foo -c "create table bar(i int generated by default as identity
> > primary key);"
> > $ pg_dump foo --exclude-table=bar
> > Segmentation fault (core dumped)

> I think the fix depends exactly on what we want the behaviour to be
> here.  Should dumpSequence() still dump out a CREATE SEQUENCE
> statement for this, or should we just ignore the sequence?  I imagine
> we should just ignore identity sequences when we're not going to dump
> the table that owns them. It looks like that would require a small
> modification in getOwnedSeqs().

Yeah, we should omit the sequence too.  I don't see any principled
reason to think that the sequence is interesting on its own.

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

On Mon, Aug 20, 2018 at 10:19:49PM -0300, Alvaro Herrera wrote:
> Yeah, we should omit the sequence too.  I don't see any principled
> reason to think that the sequence is interesting on its own.

I was just looking at this issue, or both of you are already on it?
--
Michael

On 21 August 2018 at 13:49, Michael Paquier <michael@paquier.xyz> wrote:
> On Mon, Aug 20, 2018 at 10:19:49PM -0300, Alvaro Herrera wrote:
>> Yeah, we should omit the sequence too.  I don't see any principled
>> reason to think that the sequence is interesting on its own.
>
> I was just looking at this issue, or both of you are already on it?

It's already in my head what it should look like, so I can write it,
although it might not appear until tomorrow, as I've reached the end
of my day now.

-- 
 David Rowley                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

On Tue, Aug 21, 2018 at 05:46:56PM +1200, David Rowley wrote:
> It's already in my head what it should look like, so I can write it,
> although it might not appear until tomorrow, as I've reached the end
> of my day now.

Nice to hear.  Please feel free to post at your convenient time if you
have something.
--
Michael

On 21 August 2018 at 17:46, David Rowley <david.rowley@2ndquadrant.com> wrote:
> On 21 August 2018 at 13:49, Michael Paquier <michael@paquier.xyz> wrote:
>> I was just looking at this issue, or both of you are already on it?
>
> It's already in my head what it should look like, so I can write it,
> although it might not appear until tomorrow, as I've reached the end
> of my day now.

I've attached what I had in mind.  The header comment for
getOwnedSeqs() gives me the impression this is the best place to
locate this code.

-- 
 David Rowley                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

On 21/08/2018 13:12, David Rowley wrote:
> On 21 August 2018 at 17:46, David Rowley <david.rowley@2ndquadrant.com> wrote:
>> On 21 August 2018 at 13:49, Michael Paquier <michael@paquier.xyz> wrote:
>>> I was just looking at this issue, or both of you are already on it?
>>
>> It's already in my head what it should look like, so I can write it,
>> although it might not appear until tomorrow, as I've reached the end
>> of my day now.
> 
> I've attached what I had in mind.  The header comment for
> getOwnedSeqs() gives me the impression this is the best place to
> locate this code.

Yes, that looks good to me.

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

On Tue, Aug 21, 2018 at 03:01:49PM +0200, Peter Eisentraut wrote:
> On 21/08/2018 13:12, David Rowley wrote:
>> I've attached what I had in mind.  The header comment for
>> getOwnedSeqs() gives me the impression this is the best place to
>> locate this code.
>
> Yes, that looks good to me.

This got through the TAP tests because exclude_test_table only considers
dump_test.test_table as the table to exclude when processing the dumps.
I have tried to plug in a new column into this relation but that's
proving to be a hassle with a lot of test dependencies in need to be
changed.  Trying to add a new field in full_runs is also a PITA, as that
means adding a bunch of individual markers for each test.  The nicest
way I could think of would be to be able to pass down parameters to a
given run flag, but that would require more infrastructure.  I have
checked as well if an identity column within a table's extension, and
that's working as expected.

So committed the proposed fix and back-patched down to 10.
--
Michael