BUG #15343: Segmentation fault using pg_dump with --exclude-table if table 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
Attachments:
pg_dump_seq_fix.patchapplication/octet-stream; name=pg_dump_seq_fix.patchDownload+8-4
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
Attachments:
pg_dump_identity_seq_fix.patchapplication/octet-stream; name=pg_dump_identity_seq_fix.patchDownload+14-2
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