Re: plpython improvements

Sorry, I have to revert this patch because it is causing crashes in the
plpython regression tests. Would you please run those tests, fix the
bug, and resubmit. Thanks.

---------------------------------------------------------------------------

pgman wrote:
>
> Patch applied. Thanks.
>
> ---------------------------------------------------------------------------
>
>
> Sven Suursoho wrote:
> > Hi,
> >
> >
> > Mon, 17 Apr 2006 19:20:38 +0300, Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>:
> >
> > >> > Hannu Krosing wrote:
> > >> >
> > >> >> > 1) named parameters additionally to args[]
> > >> >> > 2) return composite-types from plpython as dictionary
> > >> >> > 3) return result-set from plpython as list, iterator or generator
> > >> >> >
> > >> >> > Test script attached (patch-test.sql) but not integrated to
> > >> >> > plpython test-suite.
> > >> >>
> > >> >> If you wonder why you can't apply the patch, it is against postgres
> > >> >> 8.0.7.
> > >> >
> > >> > Can I apply it by merging it in manually, or are you working on a
> > >> > version against CVS HEAD?
> >
> > Attached patch for CVS HEAD.
> > Testscripts remained same (obviously :) but still not integrated.
> >
> >
> > --
> > Sven Suursoho
>
> [ Attachment, skipping... ]
>
> >
> > ---------------------------(end of broadcast)---------------------------
> > TIP 4: Have you searched our list archives?
> >
> > http://archives.postgresql.org
>
> --
> Bruce Momjian http://candle.pha.pa.us
> EnterpriseDB http://www.enterprisedb.com
>
> + If your life is a hard drive, Christ can be your backup. +

--
Bruce Momjian http://candle.pha.pa.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Ühel kenal päeval, N, 2006-04-27 kell 10:17, kirjutas Bruce Momjian:
> Sorry, I have to revert this patch because it is causing crashes in the
> plpython regression tests. Would you please run those tests, fix the
> bug, and resubmit. Thanks.

Where exactly does it crash ?

Please tell us the version (and buildflags) of python you are using.

There is a superfluous assert in some versions of python that has
troubled us as well.

---------------
Hannu

Hannu Krosing wrote:
> ?hel kenal p?eval, N, 2006-04-27 kell 10:17, kirjutas Bruce Momjian:
> > Sorry, I have to revert this patch because it is causing crashes in the
> > plpython regression tests. Would you please run those tests, fix the
> > bug, and resubmit. Thanks.
>
> Where exactly does it crash ?
>
> Please tell us the version (and buildflags) of python you are using.
>
> There is a superfluous assert in some versions of python that has
> troubled us as well.

Uh, I can't test plpython here because my libraries do not support it,
but I see this in the build farm:

http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=rook&dt=2006-04-27%2010:39:02

--
Bruce Momjian http://candle.pha.pa.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Hi,

Thu, 27 Apr 2006 17:17:36 +0300, Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>:

> Sorry, I have to revert this patch because it is causing crashes in the
> plpython regression tests. Would you please run those tests, fix the
> bug, and resubmit. Thanks.

Found and fixed two problems:
1) named parameters handling if there were no parameters at all
2) didn't increment counter for borrowed reference fetched from list

Also integrated tests to plpython test suite and updated existing tests to
use named parameters.

Unfortunately, there is still one problem when using unpatched python,
caused by too aggressive assert.
See
http://mail.python.org/pipermail/python-checkins/2005-August/046571.html.
I guess there should be warning somewhere as Hannu said but didn't know
where to put it.

--
Sven Suursoho

"Sven Suursoho" <sven(at)spam(dot)pri(dot)ee> writes:
> Unfortunately, there is still one problem when using unpatched python,
> caused by too aggressive assert.
> See
> http://mail.python.org/pipermail/python-checkins/2005-August/046571.html.
> I guess there should be warning somewhere as Hannu said but didn't know
> where to put it.

I don't think we are going to be able to accept a patch that causes the
server to crash when using any but a bleeding-edge copy of Python.

regards, tom lane

Sun, 30 Apr 2006 19:14:28 +0300, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:

> "Sven Suursoho" <sven(at)spam(dot)pri(dot)ee> writes:
>> Unfortunately, there is still one problem when using unpatched python,
>> caused by too aggressive assert.
>> See
>> http://mail.python.org/pipermail/python-checkins/2005-August/046571.html.
>> I guess there should be warning somewhere as Hannu said but didn't know
>> where to put it.
>
> I don't think we are going to be able to accept a patch that causes the
> server to crash when using any but a bleeding-edge copy of Python.

Actually normal python installations do not cause problem, only debugging
versions do.

Anyway, if you think that this doesn't count as an argument, there is
nothing that we can do from PG-side except drop returning SETOF as
iterator/generator and only allow return SETOF as list.

--
Sven Suursoho

Sven Suursoho wrote:
> Sun, 30 Apr 2006 19:14:28 +0300, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
>
> > "Sven Suursoho" <sven(at)spam(dot)pri(dot)ee> writes:
> >> Unfortunately, there is still one problem when using unpatched python,
> >> caused by too aggressive assert.
> >> See
> >> http://mail.python.org/pipermail/python-checkins/2005-August/046571.html.
> >> I guess there should be warning somewhere as Hannu said but didn't know
> >> where to put it.
> >
> > I don't think we are going to be able to accept a patch that causes the
> > server to crash when using any but a bleeding-edge copy of Python.
>
> Actually normal python installations do not cause problem, only debugging
> versions do.
>
> Anyway, if you think that this doesn't count as an argument, there is
> nothing that we can do from PG-side except drop returning SETOF as
> iterator/generator and only allow return SETOF as list.

Can't we detect a debug build and disable the feature?

--
Bruce Momjian http://candle.pha.pa.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Sun, 30 Apr 2006 20:48:48 +0300, Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>:

>> Sun, 30 Apr 2006 19:14:28 +0300, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
>>
>> > "Sven Suursoho" <sven(at)spam(dot)pri(dot)ee> writes:
>> >> Unfortunately, there is still one problem when using unpatched
>> python,
>> >> caused by too aggressive assert.
>> >>
>> http://mail.python.org/pipermail/python-checkins/2005-August/046571.html.
>> >> I guess there should be warning somewhere as Hannu said but didn't
>> know
>> >> where to put it.
>> >
>> > I don't think we are going to be able to accept a patch that causes
>> the
>> > server to crash when using any but a bleeding-edge copy of Python.
>>
>> Actually normal python installations do not cause problem, only
>> debugging versions do.
>>
>> Anyway, if you think that this doesn't count as an argument, there is
>> nothing that we can do from PG-side except drop returning SETOF as
>> iterator/generator and only allow return SETOF as list.
>
> Can't we detect a debug build and disable the feature?

Yes, we can, but newer python versions are already fixed.

So, what about this in configure:
if --with-python && test_iterator_app_crashes
# errcode(FEATURE_NOT_SUPPORTED), errmsg(patch your python)
disable_iterator_feature
fi

In this way we disable feature only if it is absolutely neccessary and
will give developer enough information how to fix it.

--
Sven Suursoho

"Sven Suursoho" <sven(at)spam(dot)pri(dot)ee> writes:
> So, what about this in configure:
> if --with-python && test_iterator_app_crashes
> # errcode(FEATURE_NOT_SUPPORTED), errmsg(patch your python)
> disable_iterator_feature
> fi

Testing it in configure is wrong, because there's no guarantee the same
python library will be used at runtime.

regards, tom lane

Sun, 30 Apr 2006 21:43:03 +0300, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:

> "Sven Suursoho" <sven(at)spam(dot)pri(dot)ee> writes:
>> So, what about this in configure:
>> if --with-python && test_iterator_app_crashes
>> # errcode(FEATURE_NOT_SUPPORTED), errmsg(patch your python)
>> disable_iterator_feature
>> fi
>
> Testing it in configure is wrong, because there's no guarantee the same
> python library will be used at runtime.

Yes, that's right.

Ok, will do Py_DEBUG && Py_GetVersion() check at runtime.

--
Sven Suursoho

Ühel kenal päeval, P, 2006-04-30 kell 14:43, kirjutas Tom Lane:
> "Sven Suursoho" <sven(at)spam(dot)pri(dot)ee> writes:
> > So, what about this in configure:
> > if --with-python && test_iterator_app_crashes
> > # errcode(FEATURE_NOT_SUPPORTED), errmsg(patch your python)
> > disable_iterator_feature
> > fi
>
> Testing it in configure is wrong, because there's no guarantee the same
> python library will be used at runtime.

As it is a crash bug, I can see two other ways to test:

1) do the test in startup script (maybe have pg_ctl run something)

2) test it in postmaster by running an external testprogram and see if
it crashes.

How do we handle other buggy library routines, like if some system
library crashes on divide-by-zero or similar ?

----------------
Hannu

Hi,

Sun, 30 Apr 2006 19:14:28 +0300, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:

> "Sven Suursoho" <sven(at)spam(dot)pri(dot)ee> writes:
>> Unfortunately, there is still one problem when using unpatched python,
>> caused by too aggressive assert.
>> http://mail.python.org/pipermail/python-checkins/2005-August/046571.html.
>
> I don't think we are going to be able to accept a patch that causes the
> server to crash when using any but a bleeding-edge copy of Python.

Did complete rewrite for SETOF functions: now accepts any python object
for which iter(object) returns iterable object. In this way we don't have
to deal with specific containers but can use unified python iterator API.
It means that plpython is future-proof -- whenever python introduces new
container, stored procedures already can use those without recompiling
language handler.

Also integrated with regression tests and updated existing tests to use
named parameters.

When using python interpreter with asserts enabled, generators still
crash. But I don't think that we should drop this feature because of that.
Reasons:
1) this is someone else's bug, we are using documented API correctly
2) it doesn't concern majority of users because probably there is no
asserts in production packages (tested with gentoo, ubuntu, suse). This is
true even for older python versions that are not patched.

And after all, we can document using sets, lists, tuples, iterators etc
and explicitly state that returning generator is undefined.

--
Sven Suursoho

Ühel kenal päeval, N, 2006-05-04 kell 18:21, kirjutas Sven Suursoho:
> Hi,
>
>
> Sun, 30 Apr 2006 19:14:28 +0300, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
>
> > "Sven Suursoho" <sven(at)spam(dot)pri(dot)ee> writes:
> >> Unfortunately, there is still one problem when using unpatched python,
> >> caused by too aggressive assert.
> >> http://mail.python.org/pipermail/python-checkins/2005-August/046571.html.
> >
> > I don't think we are going to be able to accept a patch that causes the
> > server to crash when using any but a bleeding-edge copy of Python.

Actually not bleeding-edge, but just version 2.4.x as distributed in
Fedora Core (and possibly in RHAS), which have assert() enabled in
python.so.

The assert there is buggy (bug
http://sourceforge.net/tracker/index.php?func=detail&aid=1257960&group_id=5470&atid=105470)

> Did complete rewrite for SETOF functions: now accepts any python object
> for which iter(object) returns iterable object. In this way we don't have
> to deal with specific containers but can use unified python iterator API.
> It means that plpython is future-proof -- whenever python introduces new
> container, stored procedures already can use those without recompiling
> language handler.
>
> Also integrated with regression tests and updated existing tests to use
> named parameters.
>
> When using python interpreter with asserts enabled, generators still
> crash. But I don't think that we should drop this feature because of that.
> Reasons:
> 1) this is someone else's bug, we are using documented API correctly
> 2) it doesn't concern majority of users because probably there is no
> asserts in production packages (tested with gentoo, ubuntu, suse). This is
> true even for older python versions that are not patched.

>From reading the bug, it seems that older versions of python also don't
have this bug, only 2.4.

> And after all, we can document using sets, lists, tuples, iterators etc
> and explicitly state that returning generator is undefined.

I think that a less confusing way of saying it would be :

"Generators crash if python version used is 2.4.x and it is compiled
with asserts.

Currently only known linux distributions to distibute such python.so
files are Fedora and possibly other RedHat distributions, while
Gentoo, Ubuntu and Suse are OK.

If you need to use generators on such a platform, compile your own
python from source and make sure that configure uses your version."

I think the patch should be commited so we can collect data about where
else the buggy version of python exists.

And if some buildfarm machines start crashing, python should be fixed
there.

----------------
Hannu

>
> I think that a less confusing way of saying it would be :
>
> "Generators crash if python version used is 2.4.x and it is compiled
> with asserts.
>
> Currently only known linux distributions to distibute such python.so
> files are Fedora and possibly other RedHat distributions, while
> Gentoo, Ubuntu and Suse are OK.

Ubuntu ships 2.4 I don't know about SuSE. 2.4 has been out for sometime
and it would be a mistake to assume that we won't run into this.

People who are "developing" in Python are going to run 2.4 because it is
the latest stable release of the language.

Sincerely,

Joshua D. Drake

> If you need to use generators on such a platform, compile your own
> python from source and make sure that configure uses your version."
>
>
> I think the patch should be commited so we can collect data about where
> else the buggy version of python exists.
>
> And if some buildfarm machines start crashing, python should be fixed
> there.
>
> ----------------
> Hannu
>
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: don't forget to increase your free space map settings
>

--

=== The PostgreSQL Company: Command Prompt, Inc. ===
Sales/Support: +1.503.667.4564 || 24x7/Emergency: +1.800.492.2240
Providing the most comprehensive PostgreSQL solutions since 1997
http://www.commandprompt.com/

Fri, 05 May 2006 19:20:55 +0300, Joshua D. Drake <jd(at)commandprompt(dot)com>:

>> I think that a less confusing way of saying it would be :
>> "Generators crash if python version used is 2.4.x and it is compiled
>> with asserts. Currently only known linux distributions to distibute
>> such python.so
>> files are Fedora and possibly other RedHat distributions, while
>> Gentoo, Ubuntu and Suse are OK.
>
> Ubuntu ships 2.4 I don't know about SuSE. 2.4 has been out for sometime
> and it would be a mistake to assume that we won't run into this.

Sure, but it is known problem and there is patch for this bug. In the
documentation we can clearly state that python2.4 with asserts enabled
causes problem and describe how it can be tested and fixed (regardless of
distribution used).

As an example of absurdity of this problem: let's assume there is known
distribution with buggy gethostbyname(). Fact, that we know about this,
shouldn't stop us developing TCP/IP applications. Especially, if there is
also patch for this bug :)

It would be real shame to prevent using generator for SETOF functions
because it is most natural match for plpgsql's "return next"

--
Sven Suursoho

>
> As an example of absurdity of this problem: let's assume there is known
> distribution with buggy gethostbyname(). Fact, that we know about this,
> shouldn't stop us developing TCP/IP applications. Especially, if there
> is also patch for this bug :)
>
> It would be real shame to prevent using generator for SETOF functions
> because it is most natural match for plpgsql's "return next"

All I was saying was that we need to account for the use of 2.4 :). So
as long as we document (as you suggest) we should be good. Sorry if I
wasn't clear.

Sincerely,

Joshua D. Drake

>
>
> --Sven Suursoho
>

--

=== The PostgreSQL Company: Command Prompt, Inc. ===
Sales/Support: +1.503.667.4564 || 24x7/Emergency: +1.800.492.2240
Providing the most comprehensive PostgreSQL solutions since 1997
http://www.commandprompt.com/

Ühel kenal päeval, R, 2006-05-05 kell 09:20, kirjutas Joshua D. Drake:
> >
> > I think that a less confusing way of saying it would be :
> >
> > "Generators crash if python version used is 2.4.x and it is compiled
> > with asserts.
> >
> > Currently only known linux distributions to distibute such python.so
> > files are Fedora and possibly other RedHat distributions, while
> > Gentoo, Ubuntu and Suse are OK.
>
> Ubuntu ships 2.4 I don't know about SuSE. 2.4 has been out for sometime
> and it would be a mistake to assume that we won't run into this.

Marko Kreen has tested the patch on Ubuntu and it is ok there.

The problem is not using 2.4, it is using 2.4 compiled with a specific set of flags.

---------------
Hannu

Sven Suursoho wrote:
> Fri, 05 May 2006 19:20:55 +0300, Joshua D. Drake <jd(at)commandprompt(dot)com>:
>
> >> I think that a less confusing way of saying it would be :
> >> "Generators crash if python version used is 2.4.x and it is compiled
> >> with asserts. Currently only known linux distributions to distibute
> >> such python.so
> >> files are Fedora and possibly other RedHat distributions, while
> >> Gentoo, Ubuntu and Suse are OK.
> >
> > Ubuntu ships 2.4 I don't know about SuSE. 2.4 has been out for sometime
> > and it would be a mistake to assume that we won't run into this.
>
> Sure, but it is known problem and there is patch for this bug. In the
> documentation we can clearly state that python2.4 with asserts enabled
> causes problem and describe how it can be tested and fixed (regardless of
> distribution used).
>
> As an example of absurdity of this problem: let's assume there is known
> distribution with buggy gethostbyname(). Fact, that we know about this,
> shouldn't stop us developing TCP/IP applications. Especially, if there is
> also patch for this bug :)
>
> It would be real shame to prevent using generator for SETOF functions
> because it is most natural match for plpgsql's "return next"

For buggy distributions of gethostbyname(), we supply our own version.
We don't just barrel ahead and tell them to fix it, if we can avoid it.
I don't see us bundling a fixed python, however. I realize it is only
Red Hat, but that is a large userbase.

I still do not know why we can't do some kind of runtime test in python
and disable this feature for 2.4 builds that have debugging enabled.
Can we do a dynamic function load test from plpython? There must be
some function that is only visible in debug builds.

--
Bruce Momjian http://candle.pha.pa.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Sat, 06 May 2006 20:38:48 +0300, Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>:

> I still do not know why we can't do some kind of runtime test in python
> and disable this feature for 2.4 builds that have debugging enabled.
> Can we do a dynamic function load test from plpython? There must be
> some function that is only visible in debug builds.

Yes, I already did research last week after discussions about that. In
unmodified Python distribution, in configure:
if --with-pydebug
define Py_DEBUG
undef NDEBUG
else
undef Py_DEBUG
define NDEBUG
fi

Unfortunately, this is not case for Fedora Core 4, where assertions are
used unconditionally. And to make things worse, there is no runtime symbol
at all to indicate whether Python is compiled with debugging/assertions
enabled (Py_DEBUG & NDEBUG are preprocessor symbols)

I see 3 options (from best to worst, IMHO)
1) document misbehaviour when using Python2.4 with assertions allowed with
reference to bug fix.
2) new configure flag to optionally allow/disallow using generator
3) drop generators

Btw, we developed returning compose types further. Basically did same as
for SETOF functions -- allow to return any Python object that conforms to
mapping protocol. Currently only dict as previously, though. But still,
ready for new fancy unforeseen Python features without recompiling PG.
Additionally, now it is possible to return compose types as Python tuples.
Will create patch whenever final decision is made about generators.

--
Sven Suursoho

Sun, 07 May 2006 12:36:59 +0300, Sven Suursoho <sven(at)spam(dot)pri(dot)ee>:

> Btw, we developed returning compose types further. Basically did same as
> for SETOF functions -- allow to return any Python object that conforms
> to mapping protocol. Currently only dict as previously, though. But
> still, ready for new fancy unforeseen Python features without
> recompiling PG.
> Additionally, now it is possible to return compose types as Python
> tuples.

Sorry, not just Python tuples but actually any object conforming to
sequence protocol (tuple, list etc)

--
Sven Suursoho

Sven Suursoho wrote:
> Sat, 06 May 2006 20:38:48 +0300, Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>:
>
> > I still do not know why we can't do some kind of runtime test in python
> > and disable this feature for 2.4 builds that have debugging enabled.
> > Can we do a dynamic function load test from plpython? There must be
> > some function that is only visible in debug builds.
>
> Yes, I already did research last week after discussions about that. In
> unmodified Python distribution, in configure:
> if --with-pydebug
> define Py_DEBUG
> undef NDEBUG
> else
> undef Py_DEBUG
> define NDEBUG
> fi
>
> Unfortunately, this is not case for Fedora Core 4, where assertions are
> used unconditionally. And to make things worse, there is no runtime symbol
> at all to indicate whether Python is compiled with debugging/assertions
> enabled (Py_DEBUG & NDEBUG are preprocessor symbols)

Can you test dynamically loading a function that is only visible in the
symbol table of debug builds, and check the return code?

In the Fedora Core 4 case, how did they make assertions always enabled?
I see the fix:

http://mail.python.org/pipermail/python-checkins/2005-August/046571.html

How did Fedora Core 4 enable asserts? I see the patch calling libc's
assert() directly, which should be enabled all the time.

--
Bruce Momjian http://candle.pha.pa.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Sun, 07 May 2006 15:58:49 +0300, Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>:

>> Unfortunately, this is not case for Fedora Core 4, where assertions are
>> used unconditionally. And to make things worse, there is no runtime
>> symbol
>> at all to indicate whether Python is compiled with debugging/assertions
>> enabled (Py_DEBUG & NDEBUG are preprocessor symbols)
>
> Can you test dynamically loading a function that is only visible in the
> symbol table of debug builds, and check the return code?

As it came out, that wouldn't help us because FC4's Python has debugging
disabled but assertions are still active. See below.

> In the Fedora Core 4 case, how did they make assertions always enabled?

Building FC4's Python rpm contains bug:
When creating package rpm from spec-file, even though Python is built
without --with-pydebug, rpmbuild loses -DNDEBUG from CFLAGS.
Can't tell how or when it happens, I'm not so good with rpm packaging
system, just looked output of compilation stage.

So, we are back on square one...

--
Sven Suursoho

Added to TODO:

o Allow PL/python to composite types and result sets
once buggy assert-enabled versions of python can be detected

http://archives.postgresql.org/pgsql-patches/2006-04/msg00087$

---------------------------------------------------------------------------

Hannu Krosing wrote:
> ?hel kenal p?eval, N, 2006-05-04 kell 18:21, kirjutas Sven Suursoho:
> > Hi,
> >
> >
> > Sun, 30 Apr 2006 19:14:28 +0300, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
> >
> > > "Sven Suursoho" <sven(at)spam(dot)pri(dot)ee> writes:
> > >> Unfortunately, there is still one problem when using unpatched python,
> > >> caused by too aggressive assert.
> > >> http://mail.python.org/pipermail/python-checkins/2005-August/046571.html.
> > >
> > > I don't think we are going to be able to accept a patch that causes the
> > > server to crash when using any but a bleeding-edge copy of Python.
>
> Actually not bleeding-edge, but just version 2.4.x as distributed in
> Fedora Core (and possibly in RHAS), which have assert() enabled in
> python.so.
>
> The assert there is buggy (bug
> http://sourceforge.net/tracker/index.php?func=detail&aid=1257960&group_id=5470&atid=105470)
>
> > Did complete rewrite for SETOF functions: now accepts any python object
> > for which iter(object) returns iterable object. In this way we don't have
> > to deal with specific containers but can use unified python iterator API.
> > It means that plpython is future-proof -- whenever python introduces new
> > container, stored procedures already can use those without recompiling
> > language handler.
> >
> > Also integrated with regression tests and updated existing tests to use
> > named parameters.
> >
> > When using python interpreter with asserts enabled, generators still
> > crash. But I don't think that we should drop this feature because of that.
> > Reasons:
> > 1) this is someone else's bug, we are using documented API correctly
> > 2) it doesn't concern majority of users because probably there is no
> > asserts in production packages (tested with gentoo, ubuntu, suse). This is
> > true even for older python versions that are not patched.
>
> >From reading the bug, it seems that older versions of python also don't
> have this bug, only 2.4.
>
> > And after all, we can document using sets, lists, tuples, iterators etc
> > and explicitly state that returning generator is undefined.
>
> I think that a less confusing way of saying it would be :
>
> "Generators crash if python version used is 2.4.x and it is compiled
> with asserts.
>
> Currently only known linux distributions to distibute such python.so
> files are Fedora and possibly other RedHat distributions, while
> Gentoo, Ubuntu and Suse are OK.
>
> If you need to use generators on such a platform, compile your own
> python from source and make sure that configure uses your version."
>
>
> I think the patch should be commited so we can collect data about where
> else the buggy version of python exists.
>
> And if some buildfarm machines start crashing, python should be fixed
> there.
>
> ----------------
> Hannu
>
>
>

--
Bruce Momjian http://candle.pha.pa.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Quoting Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>:

>
> Added to TODO:
>
> o Allow PL/python to composite types and result sets
> once buggy assert-enabled versions of python can be
detected

I actually tried to work other way around -- get all known buggy systems
fixed:
1) Debugging Python version: bug acknowledged, v2.5 fixed, v2.4 waiting
(http://sourceforge.net/tracker/index.php?func=detail&aid=1483133&group_id=5470&atid=105470)
2) Fedora: >=v5 fixed, will be backported to version 4 also
(https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=192592)

If this approach is not satisfactionary and assertion predetection is
still requested, only solution I see is to catch signal SIGABRT, set
flag, longjmp back and report error.
But I really don't like it.

--
Sven Suursoho

Sven <sven(at)spam(dot)pri(dot)ee> writes:
> If this approach is not satisfactionary and assertion predetection is
> still requested, only solution I see is to catch signal SIGABRT, set
> flag, longjmp back and report error.
> But I really don't like it.

No, ignoring SIGABRT is surely right out.

It suddenly strikes me though that we are being too picky. The reason
(which I admit having forgotten) is that plpython only comes in an
untrusted variant. That means that anyone writing plpython functions
is already a database superuser, and has much more direct means of
crashing the backend if he chooses to. So the idea that this python bug
constitutes a security threat seems overblown.

If there ever is a future Python release with a new sandbox mechanism,
enabling resurrection of the trusted language, it would presumably
contain the bug fix.

Obviously we should document the problem in the plpython documentation
("don't try to use this feature with python versions before XXX"), but
I'm not any longer convinced that we have to reject this patch on
security grounds.

[ Whether the patch is any good is a separate question ;-). I have
not reviewed it. ]

regards, tom lane

On Sat, 17 Jun 2006 18:47:46 +0300, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Obviously we should document the problem in the plpython documentation
> ("don't try to use this feature with python versions before XXX"), but
> I'm not any longer convinced that we have to reject this patch on
> security grounds.

Here it is, updated against CVS HEAD.
Will create documentation after acceptance of patch.

--
Sven Suursoho

Quoting Sven Suursoho <public(at)spam(dot)pri(dot)ee>:

> On Sat, 17 Jun 2006 18:47:46 +0300, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> > Obviously we should document the problem in the plpython documentation
> > ("don't try to use this feature with python versions before XXX"), but
> > I'm not any longer convinced that we have to reject this patch on
> > security grounds.
>
> Here it is, updated against CVS HEAD.
> Will create documentation after acceptance of patch.

And here it comes again :)
Fedora has fixed bug that caused initial rejection
(https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=192592).

--
Sven Suursoho

Sven Suursoho wrote:
> Quoting Sven Suursoho <public(at)spam(dot)pri(dot)ee>:
>
>
> > On Sat, 17 Jun 2006 18:47:46 +0300, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >
> > > Obviously we should document the problem in the plpython documentation
> > > ("don't try to use this feature with python versions before XXX"), but
> > > I'm not any longer convinced that we have to reject this patch on
> > > security grounds.
> >
> > Here it is, updated against CVS HEAD.
> > Will create documentation after acceptance of patch.
>
> And here it comes again :)
> Fedora has fixed bug that caused initial rejection
> (https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=192592).

Great. Please supply documentation and it will be applied. Thanks.

--
Bruce Momjian bruce(at)momjian(dot)us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Hi,

Quoting Bruce Momjian <bruce(at)momjian(dot)us>:

> Great. Please supply documentation and it will be applied. Thanks.

Here it comes, including src+doc patches.
Updated sources according to Michael Fuhr's comments and fixed one FIXME.

Please check documentation patch thoroughly as I'm not native English
speaker nor didn't manage to generate documentation from SGML sources (
openjade:postgres.sgml:3:55:W: cannot generate system identifier for public
text "-//OASIS//DTD DocBook V4.2//EN")

--
Sven Suursoho

Sven Suursoho wrote:
> Hi,
>
>
> Quoting Bruce Momjian <bruce(at)momjian(dot)us>:
>
> > Great. Please supply documentation and it will be applied. Thanks.
>
> Here it comes, including src+doc patches.
> Updated sources according to Michael Fuhr's comments and fixed one FIXME.
>
> Please check documentation patch thoroughly as I'm not native English
> speaker nor didn't manage to generate documentation from SGML sources (
> openjade:postgres.sgml:3:55:W: cannot generate system identifier for public
> text "-//OASIS//DTD DocBook V4.2//EN")

Patch applied. Thanks. I improved your documentation wording, updated
version attached.

--
Bruce Momjian bruce(at)momjian(dot)us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +