Re: Multiple pg_waldump --rmgr options

From: Daniel Gustafsson <daniel(at)yesql(dot)se>
To: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Multiple pg_waldump --rmgr options
Date: 2021-06-30 21:14:56
Message-ID: 78A82038-8B2D-46E1-9E06-193FA9B3AF73@yesql.se
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On 30 Jun 2021, at 22:39, Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:

> In general, I think it's weird that the latest option wins. If you specify the same option multiple times, and it's not something like --rmgr or --table where it makes sense, it's most likely user error. Printing an error would be nicer than ignoring all but the last instance. But I'm not going to try changing that now.

AFAIK, the traditional "defense" for it when building a commandline with
scripts which loop over input, to avoid the need for any data structure holding
the options for deduplication. No idea how common that is these days, but I've
seen it in production in the past for sure.

> I tried to be defensive against WAL records with bogus xl_rmid values here:
>
>> @@ -1098,8 +1100,9 @@ main(int argc, char **argv)
>> }
>> /* apply all specified filters */
>> - if (config.filter_by_rmgr != -1 &&
>> - config.filter_by_rmgr != record->xl_rmid)
>> + if (config.filter_by_rmgr_enabled &&
>> + (record->xl_rmid < 0 || record->xl_rmid > RM_MAX_ID ||
>> + !config.filter_by_rmgr[record->xl_rmid]))
>> continue;
>
> But looking closer, that's pointless. We use record->xl_rmid directly as array index elsewhere, and that's OK because ValidXLogRecordHeader() checks that xl_rmid <= RM_MAX_ID. And the 'xl_rmid < 0' check is unnecessary because the field is unsigned. So I'll remove those, and commit this tomorrow.

+1

--
Daniel Gustafsson https://vmware.com/

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Álvaro Herrera 2021-06-30 21:32:40 Re: Partitioned index can be not dumped
Previous Message Daniel Gustafsson 2021-06-30 20:46:37 Re: SSL/TLS instead of SSL in docs