Thread: Re: [COMMITTERS] pgsql: Get rid of the global variable holding the error state

On lör, 2011-01-22 at 16:36 -0500, Tom Lane wrote:
> Peter Eisentraut <peter_e@gmx.net> writes:
> > Get rid of the global variable holding the error state
> 
> The buildfarm doesn't like this patch one bit.

I have verified your adjustments and fixed up the rest.

Peter Eisentraut <peter_e@gmx.net> writes:
> On lör, 2011-01-22 at 16:36 -0500, Tom Lane wrote:
>> Peter Eisentraut <peter_e@gmx.net> writes:
>>> Get rid of the global variable holding the error state

>> The buildfarm doesn't like this patch one bit.

> I have verified your adjustments and fixed up the rest.

I was a bit disturbed by the fact that fixing only one of the four
variant files was enough to turn the whole buildfarm green.  Are the
other three cases even needed anymore?  If so, how can we get some
coverage on them?
        regards, tom lane

On ons, 2011-01-26 at 17:47 -0500, Tom Lane wrote:
> Peter Eisentraut <peter_e@gmx.net> writes:
> > On lör, 2011-01-22 at 16:36 -0500, Tom Lane wrote:
> >> Peter Eisentraut <peter_e@gmx.net> writes:
> >>> Get rid of the global variable holding the error state
> 
> >> The buildfarm doesn't like this patch one bit.
> 
> > I have verified your adjustments and fixed up the rest.
> 
> I was a bit disturbed by the fact that fixing only one of the four
> variant files was enough to turn the whole buildfarm green.  Are the
> other three cases even needed anymore?  If so, how can we get some
> coverage on them?

This is explained in plpython/expected/README.  As you can see there, it
would need some careful attention from buildfarm code and participants
to cover all that.

Peter Eisentraut <peter_e@gmx.net> writes:
> On ons, 2011-01-26 at 17:47 -0500, Tom Lane wrote:
>> I was a bit disturbed by the fact that fixing only one of the four
>> variant files was enough to turn the whole buildfarm green.  Are the
>> other three cases even needed anymore?  If so, how can we get some
>> coverage on them?

> This is explained in plpython/expected/README.  As you can see there, it
> would need some careful attention from buildfarm code and participants
> to cover all that.

Hmmm ... well, the fact that we have zero coverage on the first two
variants definitely seems surprising and scary to me.  Why aren't those
getting hit by the non-C-locale buildfarm runs?
        regards, tom lane

On 27/01/11 00:15, Tom Lane wrote:
> Peter Eisentraut <peter_e@gmx.net> writes:
>> On ons, 2011-01-26 at 17:47 -0500, Tom Lane wrote:
>>> I was a bit disturbed by the fact that fixing only one of the four
>>> variant files was enough to turn the whole buildfarm green.  Are the
>>> other three cases even needed anymore?  If so, how can we get some
>>> coverage on them?
> 
>> This is explained in plpython/expected/README.  As you can see there, it
>> would need some careful attention from buildfarm code and participants
>> to cover all that.
> 
> Hmmm ... well, the fact that we have zero coverage on the first two
> variants definitely seems surprising and scary to me.  Why aren't those
> getting hit by the non-C-locale buildfarm runs?

Looking at the README, you get the basic output file if you have server
encoding != SQL_ASCII and client encoding UTF8, which is what I was
testing with.

_0 is when you have server encoding != SQL_ASCII and client encoding !=
UTF8, which I'm not sure how popular of a setup is in the buildfarm (or
maybe by sheer luck it didn't break, dunno).

_2 is only Python 2.2, but I tried: with Python 2.2 there's a whole lot
of regression tests that fail. The last release of 2.2 is April 2003,
maybe it's time to forget about that particular dinosaur?

When coding I was running tests with Pythons 2.3 to 3.1 and trying to
keep the stuff working with these versions, as the last 2.3 release was
in March 2008.

_3 is the variant file you get if your server is SQL_ASCII and you have
a non-ancient Python, which I guess is the config quote a few buildfarm
animals has.

So three things:* I should test with SQL_ASCII* we might want to check if the the _0 variant file needs updates* maybe
it'stime to stop supporting Python 2.2
 

Cheers,
Jan

Jan Urbański <wulczer@wulczer.org> writes:
> _2 is only Python 2.2, but I tried: with Python 2.2 there's a whole lot
> of regression tests that fail. The last release of 2.2 is April 2003,
> maybe it's time to forget about that particular dinosaur?

Well, there's little point in carrying an incorrect expected-file, so
unless someone is prepared to test the case regularly, I'd agree with
deleting that file.  We could just add a note that the expected outputs
are intended for 2.3 and up and you'll get some error-message-wording
discrepancies with older python versions.  The differences between _2
and _3 seem small enough for people to handle by visual inspection if
they really want to test the case.
        regards, tom lane

On tor, 2011-01-27 at 00:25 +0100, Jan Urbański wrote:
> _2 is only Python 2.2, but I tried: with Python 2.2 there's a whole
> lot of regression tests that fail. The last release of 2.2 is April
> 2003, maybe it's time to forget about that particular dinosaur?

I agree that we should remove the plpython_unicode_2.out expected file
since none of the other expected files support 2.2.

I would also support dropping 2.2 support at the source code level.  It
would only save a few lines of compatibility code, but we can't support
everything forever, and I think it'd be better to draw a clear line than
maintaining the current state of works-but-no-test-support.