Re: Logical Replication WIP

On 13/12/16 03:26, Petr Jelinek wrote:
> On 13/12/16 02:41, Andres Freund wrote:
>> On 2016-12-10 08:48:55 +0100, Petr Jelinek wrote:
>>
>>> +static List *
>>> +OpenTableList(List *tables)
>>> +{
>>> + List *relids = NIL;
>>> + List *rels = NIL;
>>> + ListCell *lc;
>>> +
>>> + /*
>>> + * Open, share-lock, and check all the explicitly-specified relations
>>> + */
>>> + foreach(lc, tables)
>>> + {
>>> + RangeVar *rv = lfirst(lc);
>>> + Relation rel;
>>> + bool recurse = interpretInhOption(rv->inhOpt);
>>> + Oid myrelid;
>>> +
>>> + rel = heap_openrv(rv, ShareUpdateExclusiveLock);
>>> + myrelid = RelationGetRelid(rel);
>>> + /* filter out duplicates when user specifies "foo, foo" */
>>> + if (list_member_oid(relids, myrelid))
>>> + {
>>> + heap_close(rel, ShareUpdateExclusiveLock);
>>> + continue;
>>> + }
>>
>> This is a quadratic algorithm - that could bite us... Not sure if we
>> need to care. If we want to fix it, one approach owuld be to use
>> RangeVarGetRelid() instead, and then do a qsort/deduplicate before
>> actually opening the relations.
>>
>
> I guess it could get really slow only with big inheritance tree, I'll
> look into how much work is the other way of doing things (this is not
> exactly hot code path).
>

Actually looking at it, it only processes user input so I don't think
it's very problematic in terms of performance. You'd have to pass many
thousands of tables in single DDL to notice.

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