On Mon, Sep 16, 2019 at 11:09 AM Kuntal Ghosh <kuntalghosh.2...@gmail.com> wrote: > Okay. In that case, we need to rethink the cases for multi-inserts and > non-inlace updates both of which currently inserts multiple undo > record corresponding to a single WAL record. For multi-inserts, it can > be solved easily by moving all the offset information in the payload. > But, for non-inlace updates, we insert one undo record for the update > and one for the insert. Wondering whether we've to insert two WAL > records - one for update and one for the new insert.
No, I think the solution is to put the information about both halves of the non-in-place update in the same undo record. I think the only reason why that's tricky is because we've got two block numbers and two offsets, and the only reason that's a problem is because UnpackedUndoRecord only has one field for each of those things, and that goes right back to Heikki's comments about the format not being flexible enough. If you see some other problem, it would be interesting to know what it is. One thing I've been thinking about is: suppose that you're following the undo chain for a tuple and you come to a non-in-place update record. Can you get confused? I don't think so, because you can compare the TID for which you're following the chain to the new TID and the old TID in the record and it should match one or the other but not both. But I don't think you even really need to do that much: if you started with a deleted item, the first thing in the undo chain has to be a delete or non-in-place update that got rid of it. And if you started with a non-deleted item, then the beginning of the undo chain, if it hasn't been discarded yet, will be the insert or non-in-place update that created it. There's nowhere else that you can hit a non-in-place update, and no room (that I can see) for any ambiguity. It seems to me that zheap went wrong in ending up with separate undo types for in-place and non-in-place updates. Why not just have ONE kind of undo record that describes an update, and allow that update to have either one TID or two TIDs depending on which kind of update it is? There may be a reason, but I don't know what it is, unless it's just that the UnpackedUndoRecord idea that I invented wasn't flexible enough and nobody thought of generalizing it. Curious to hear your thoughts on this. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company