Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE

From: Michael Meskes <meskes(at)postgresql(dot)org>
To: Michael Paquier <michael(at)paquier(dot)xyz>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Andrew Dunstan <andrew(at)dunslane(dot)net>, Bruce Momjian <bruce(at)momjian(dot)us>, Peter Geoghegan <pg(at)bowt(dot)ie>, "kuroda(dot)hayato(at)fujitsu(dot)com" <kuroda(dot)hayato(at)fujitsu(dot)com>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: ECPG bug fix: DECALRE STATEMENT and DEALLOCATE, DESCRIBE
Date: 2021-08-16 10:06:16
Message-ID: 54abf356a90d35ce54c7e5050b8247fd8b69721f.camel@postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> > (1) There should be no output to stderr in the tests.  Why isn't
> > this
> > message being caught and redirected to the normal test output file?
>
> These are generated during the compilation of the tests with the
> pre-processor, so that's outside the test runs.

This is actually a deeper issue, we have no test for the compiler
itself, other than the source code it generates. We do not test
warnings or errors thrown by it. The topic has come up ages ago and we
simply removed the test that generated the (planned) warning message.

> > (2) This message is both unintelligible and grammatically
> > incorrect.
>
> Yeah, debugging such tests would be more helpful if the name of the
> DECLARE statement is included, at least.  Those messages being
> generated is not normal anyway, which is something coming from the
> tests as a typo with the connection name of stmt_3.
>
> Michael, what do you think about the attached?

I think what Tom was saying is that it should be either "is overwritten
with" or "is rewritten to", but you raise a very good point. Adding the
statement name makes the message better. I fully agree. However, it
should be the other way round, the DECLARE STATEMENT changes the
connection that is used.

You patch removes the warning but by doing that also removes the
feature that is being tested.

I'm not sure what's the best way to go about it, Shall we accept to not
test this particular feature and remove the warning? After all this is
not the way the statement should be used, hence the warning. Or should
be keep it in and redirect the warning? In that case, we would also
lose other warnings that are not planned, though.

Any comments?

Michael
--
Michael Meskes
Michael at Fam-Meskes dot De
Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2021-08-16 10:28:24 Re: Window Function "Run Conditions"
Previous Message Peter Eisentraut 2021-08-16 09:51:50 Non-decimal integer literals