Re: sequences and pg_upgrade

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>
Cc: Anastasia Lubennikova <a(dot)lubennikova(at)postgrespro(dot)ru>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: sequences and pg_upgrade
Date: 2016-11-02 06:34:20
Message-ID: CAB7nPqTBEWXV2C+eENp9G5QOe2+fU0ZGfRrkASm7HLjd7S9hiQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Oct 31, 2016 at 9:53 PM, Peter Eisentraut
<peter(dot)eisentraut(at)2ndquadrant(dot)com> wrote:
> On 9/30/16 12:50 PM, Anastasia Lubennikova wrote:
>> The patches are good, no complaints.
>> But again, I have the same question.
>> I was confused, why do we always dump sequence data,
>> because I'd overlooked the --sequence-data key. I'd rather leave this
>> option,
>> because it's quite non intuitive behaviour...
>> /* dump sequence data even in schema-only mode */
>
> Here are rebased patches.
>
> Regarding your question: The initial patch had a separate option for
> this behavior, which was then used by pg_upgrade. It was commented that
> this option is not useful outside of pg_upgrade, so it doesn't need to
> be exposed as a user-facing option. I agreed with that and removed the
> option. We can always add the option back easily if someone really
> wants it, but so far no use case has been presented. So I suggest we
> proceed with this proposal ignoring whether this option is exposed or not.

I had a look at those fresh patches, and 0001 looks like a good thing.
This makes the separation between sequences and table data dump
cleaner. I ran some tests with pg_upgrade and 0002, and things are
clear. And +1 for the way done in the patch, aka no options of pg_dump
exposed to user, still keep the option tracking as a separate value.

One small thing here:
static void
-getTableData(DumpOptions *dopt, TableInfo *tblinfo, int numTables, bool oids)
+getTableData(DumpOptions *dopt, TableInfo *tblinfo, int numTables,
bool oids, char relkind)
{
int i;

for (i = 0; i < numTables; i++)
{
- if (tblinfo[i].dobj.dump & DUMP_COMPONENT_DATA)
+ if (tblinfo[i].dobj.dump & DUMP_COMPONENT_DATA &&
+ (!relkind || tblinfo[i].relkind == relkind))
makeTableDataInfo(dopt, &(tblinfo[i]), oids)

One idea here would be to have an extra routine, getSequenceData and
not extend getTableData() with relkind as extra argument. I am fine
with the way patch does things, so I just switched the patch as ready
for committer.
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Craig Ringer 2016-11-02 06:54:47 Re: Using a latch between a background worker process and a thread
Previous Message Ashutosh Sharma 2016-11-02 05:38:19 Re: Microvacuum support for Hash Index