Re: [HACKERS] 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