Обсуждение: MERGE regress test
Hi,
A quick question on merge regress-test
https://github.com/postgres/postgres/blob/REL_15_STABLE/src/test/regress/expected/merge.out#L846
should there be an ERROR or comment needs a fix? What’s the expected behavior?
Regards
Teja
Вложения
On 2022-Nov-16, Teja Mupparti wrote: > A quick question on merge regress-test > > https://github.com/postgres/postgres/blob/REL_15_STABLE/src/test/regress/expected/merge.out#L846 j > should there be an ERROR or comment needs a fix? What's the expected behavior? Hmm, good find. As I recall, I was opposed to the idea of throwing an error if the WHEN expression writes to the database, and the previous implementation had some hole, so I just ripped it out after discussing it; but I evidently failed to notice this test case about it. However, I can't find any mailing list discussion about this point. Maybe I just asked Simon off-list about it. IMO just deleting that test is a sufficient fix. -- Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/ Tom: There seems to be something broken here. Teodor: I'm in sackcloth and ashes... Fixed. http://archives.postgresql.org/message-id/482D1632.8010507@sigaev.ru
On 2022-Nov-16, Alvaro Herrera wrote: > Hmm, good find. As I recall, I was opposed to the idea of throwing an > error if the WHEN expression writes to the database, and the previous > implementation had some hole, so I just ripped it out after discussing > it; but I evidently failed to notice this test case about it. Ah, I found out what happened, and my memory as usual is betraying me. This was changed before I was involved with the patch at all: Pavan changed it between his v18[1] and v19[2]: if (action->whenqual) { - int64 startWAL = GetXactWALBytes(); - bool qual = ExecQual(action->whenqual, econtext); - - /* - * SQL Standard says that WHEN AND conditions must not - * write to the database, so check we haven't written - * any WAL during the test. Very sensible that is, since - * we can end up evaluating some tests multiple times if - * we have concurrent activity and complex WHEN clauses. - * - * XXX If we had some clear form of functional labelling - * we could use that, if we trusted it. - */ - if (startWAL < GetXactWALBytes()) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot write to database within WHEN AND condition"))); This is what Peter Geoghegan had to say about it at the time: > This needs to go. Apart from the fact that GetXactWALBytes() is buggy > (it returns int64 for the unsigned type XLogRecPtr), the whole idea > just seems unnecessary. I don't see why this is any different to using > a volatile function in a regular UPDATE. Pavan just forgot to remove the test. I'll do so now. [1] https://postgr.es/m/CABOikdPFCcgp7=zoN4M=y0TefW4Q9dPAU+Oy5jN5A+hWYdnvNg@mail.gmail.com [2] https://postgr.es/m/CABOikdOUoaXt1H885TC_cA1LoErEejqdVDZqG62rQkiZPPyg0Q@mail.gmail.com -- Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/ "No tengo por qué estar de acuerdo con lo que pienso" (Carlos Caszeli)
On 2022-Nov-18, Alvaro Herrera wrote: > Pavan just forgot to remove the test. I'll do so now. Done now. Thanks for reporting. -- Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/ "La conclusión que podemos sacar de esos estudios es que no podemos sacar ninguna conclusión de ellos" (Tanenbaum)