RE: Logical Replication of sequences

Поиск
Список
Период
Сортировка
От Hayato Kuroda (Fujitsu)
Тема RE: Logical Replication of sequences
Дата
Msg-id OSCPR01MB14966325AA3494AF164E11B9CF526A@OSCPR01MB14966.jpnprd01.prod.outlook.com
обсуждение исходный текст
Ответ на Re: Logical Replication of sequences  (vignesh C <vignesh21@gmail.com>)
Список pgsql-hackers
Dear Vignesh,

I played with your patch and found something.

01.
In LogicalRepSyncSequences() and GetSubscriptionRelations(), there is a possibility
that the sequence on the subscriber could be dropped before opens that.
This can cause `could not open relation with OID %u` error, which is not user-friendly.
Can we avoid that? Even if it is difficult we should add ereport().

02.
```
        /*
         * Check that our sequencesync worker has permission to insert into
         * the target sequence.
         */
        aclresult = pg_class_aclcheck(RelationGetRelid(sequence_rel), GetUserId(),
                                      ACL_INSERT);
        if (aclresult != ACLCHECK_OK)
            aclcheck_error(aclresult,
                           get_relkind_objtype(sequence_rel->rd_rel->relkind),
                           seqname);
```

Hmm, but upcoming SetSequence() needs UPDATE privilege. I feel it should be checked.

03.
Similar with 1, sequences can be dropped just before entering copy_sequences().
This can cause `cache lookup failed for sequence` error, which cannot be translated.
Can we avoid that or change the error-function to erport()?

04.
```
                if (message_level_is_interesting(DEBUG1))
                    ereport(DEBUG1,
                            errmsg_internal("logical replication synchronization for subscription \"%s\", sequence
\"%s.%s\"has finished",
 
                                            MySubscription->name,
                                            seqinfo->nspname,
                                            seqinfo->seqname));
```

I feel no need to add if-statement because we do not touch additional data here.

05.
```
    list_free_deep(sequences_to_copy);
```

IIUC, this function free's each elements and list itself, but they do no-op for
attributes of elements. Can we pfree() for seqname and nspname?

Best regards,
Hayato Kuroda
FUJITSU LIMITED


В списке pgsql-hackers по дате отправления: