On 2021-Jan-24, Julien Rouhaud wrote: > While working on pg14 compatibility for an extension relying on an apparently > uncommon combination of FOR UPDATE and stored function calls, I hit some new > Asserts introduced in 866e24d47db (Extend amcheck to check heap pages): > > + /* > + * Do not allow tuples with invalid combinations of hint bits to be > placed > + * on a page. These combinations are detected as corruption by the > + * contrib/amcheck logic, so if you disable one or both of these > + * assertions, make corresponding changes there. > + */ > + Assert(!((tuple->t_data->t_infomask & HEAP_XMAX_LOCK_ONLY) && > + (tuple->t_data->t_infomask2 & HEAP_KEYS_UPDATED))); > > > I attach a simple self contained script to reproduce the problem, the last > UPDATE triggering the Assert.
Maybe we should contest the idea that this is a sensible thing to Assert against. AFAICS this was originally suggested here: https://www.postgresql.org/message-id/flat/CAFiTN-syyHc3jZoou51v0SR8z0POoNfktqEO6MaGig4YS8mosA%40mail.gmail.com#ad215d0ee0606b5f67bbc57d011c96b8 and it appears now to have been a bad idea. If I recall correctly, HEAP_KEYS_UPDATED is supposed to distinguish locks/updates that don't modify the key columns from those that do. Since SELECT FOR UPDATE stands for a future update that may modify arbitrary portions of the tuple (including "key" columns), then it produces that bit, just as said UPDATE or a DELETE; as opposed to SELECT FOR NO KEY UPDATE which stands for a future UPDATE that will only change columns that aren't part of any keys. So I think that I misspoke earlier in this thread when I said this is a bug, and that the right fix here is to remove the Assert() and change amcheck to match. Separately, maybe it'd also be good to have a test case based on Julien's SQL snippet that produces this particular infomask combination (and other interesting combinations) and passes them through VACUUM etc to see that everything behaves correctly. You could also argue the HEAP_KEYS_UPDATED is a misnomer and that we'd do well to change its name, and update README.tuplock. -- Álvaro Herrera 39°49'30"S 73°17'W Bob [Floyd] used to say that he was planning to get a Ph.D. by the "green stamp method," namely by saving envelopes addressed to him as 'Dr. Floyd'. After collecting 500 such letters, he mused, a university somewhere in Arizona would probably grant him a degree. (Don Knuth)
