On Tue, Apr 30, 2019 at 8:44 PM Robert Haas <robertmhaas@gmail.com> wrote:
> UndoRecInfo looks a bit silly, I think. Isn't index just the index of
> this entry in the array? You can always figure that out by ptr -
> array_base. Instead of having UndoRecPtr urp in this structure, how
> about adding that to UnpackedUndoRecord? When inserting, caller
> leaves it unset and InsertPreparedUndo sets it; when retrieving,
> UndoFetchRecord or UndoRecordBulkFetch sets it. With those two
> changes, I think you can get rid of UndoRecInfo entirely and just
> return an array of UnpackedUndoRecords. This might also eliminate the
> need for an 'urp' member in PreparedUndoSpace.
>
Yeah, at least in this patch it looks silly. Actually, I added that
index to make the qsort stable when execute_undo_action sorts them in
block order. But, as part of this patch we don't have that processing
so either we can remove this structure completely as you suggested but
undo processing patch has to add that structure or we can just add
comment that why we added this index field.
I am ok with other comments and will work on them.
--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com