Horton

Thread

Commits

  1. Remove useless MERGE test

  1. MERGE regress test

    Teja Mupparti <tejeswar.mupparti@microsoft.com> — 2022-11-16T05:46:04Z

    Hi,



A quick question on merge regress-test



https://github.com/postgres/postgres/blob/REL_15_STABLE/src/test/regress/expected/merge.out#L846<https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fpostgres%2Fpostgres%2Fblob%2FREL_15_STABLE%2Fsrc%2Ftest%2Fregress%2Fexpected%2Fmerge.out%23L846&data=05%7C01%7CTejeswar.Mupparti%40microsoft.com%7Ca57053f63d654a01c1f608dac77bbf59%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C638041631582821358%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=9T3%2FhFcEK9dhslD6suiz6%2B5kbcPDIaZ24uDDy8WUbqs%3D&reserved=0>



should there be an ERROR or comment needs a fix? What's the expected behavior?



Regards

Teja

[Text  Description automatically generated]

  2. Re: MERGE regress test

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2022-11-16T17:37:14Z

    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

  3. Re: MERGE regress test

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2022-11-18T19:08:05Z

    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)

  4. Re: MERGE regress test

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2022-11-22T10:29:08Z

    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)