> > appendStringInfo(buf, "xid %u (db/rel) %u/%u ", > xlrec->locks[i].xid, xlrec->locks[i].dbOid, > xlrec->locks[i].relOid); > > > As I said, I don't know whether it's relevant to perform these changes > or not.
Maybe, I'm not against doing that. But if we do that, I don't think we need to add the "(db/rel)" bit - we don't do that elsewhere, so why here?
Yes, you spotted a bad copy/paste from me ; "(db/rel)" should bit should not be there. Sorry. Why that error so ? I admit I tried some changes on a local git branch of mine with another format helping the user to understand what ids really were. (each line containing ids had "(ts/db/rel)" or "(db/rel)" inside. On COMMIT messages, there also was only one "(db/rels)" bit. This was just an answer to Peter proposal on a different format. I wanted to keep A/B/C-like format and help DBA to understand what it was using a prefix string like (ts/db/rel). But as discussions were going on, I figured out it might be a bad idea so I did not submit.
And if we adopt the same format, should that also include the tablespace? Although, maybe for locks that doesn't make much sense.
I agree that, in a way, it may be a good idea to use that A/B/C format every where each time we use ids. It would make sense for the dba to know what kind of ids are involved if we have partial ones (A/B or B/C).
I don't know the code enough to say whether it would be difficult to retrieve each time the tablespace or not.
There's also lines with base/db/rel ... So there's some different format. Anyway, I handle all of them (I hope so) in my processing script. So keeping the things just as they are (out of the initial COPY patch that should be applied *to me*) is no problem for me.
On the other hand, there's a consensus not to go further than the initial patch.