> On 30 Jun 2021, at 22:39, Heikki Linnakangas <hlinnaka@iki.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
notsomething like --rmgr or --table where it makes sense, it's most likely user error. Printing an error would be nicer
thanignoring 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
isunsigned. So I'll remove those, and commit this tomorrow.
+1
--
Daniel Gustafsson https://vmware.com/