On Wed, Jul 24, 2019 at 11:28 AM Rushabh Lathia <rushabh.lat...@gmail.com> wrote: > > Hi, > > I have stated review of > 0008-Provide-interfaces-to-store-and-fetch-undo-records.patch, here are few > quick comments.
Thanks for the review, I will work on them soon and post the updated patch along with other comments. I have noticed some comments are pointing to the code which is not part of this patch for example > > 5) In function UndoBlockGetFirstUndoRecord() below code: > > /* Calculate the size of the partial record. */ > partial_rec_size = UndoRecordHeaderSize(phdr->uur_info) + > phdr->tuple_len + phdr->payload_len - > phdr->record_offset; > > can directly use UndoPagePartialRecSize(). UndoBlockGetFirstUndoRecord is added under 0014 patch, I think you got confused because this code is in undoaccess.c file. But actually later patch set added some code under undoaccess.c. Basically, this comment needs to be fixed but under another patch. I am pointing out so that we don't miss this. > 8) > > /* > * Defines the number of times we try to wait for rollback hash table to get > * initialized. After these many attempts it will return error and the user > * can retry the operation. > */ > #define ROLLBACK_HT_INIT_WAIT_TRY 60 > > %s/error/an error > This macro also added under 0014. -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com