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 по дате отправления: