plpython improvements

Started by Sven Suursohoover 20 years ago35 messagespatches
Jump to latest
#1Sven Suursoho
sven@spam.pri.ee

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.

--
Sven Suursoho

Attachments:

patch-test.sqlapplication/octet-stream; name=patch-test.sqlDownload
functions.sqlapplication/octet-stream; name=functions.sqlDownload
plpython.patchtext/x-patch; name=plpython.patchDownload+249-27
#2Hannu Krosing
hannu@tm.ee
In reply to: Sven Suursoho (#1)
Re: plpython improvements

Ühel kenal päeval, L, 2006-04-15 kell 17:59, kirjutas Sven Suursoho:

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.

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

#3Bruce Momjian
bruce@momjian.us
In reply to: Hannu Krosing (#2)
Re: plpython improvements

Hannu Krosing wrote:

?hel kenal p?eval, L, 2006-04-15 kell 17:59, kirjutas Sven Suursoho:

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?

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

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

#4Sven Suursoho
sven@spam.pri.ee
In reply to: Bruce Momjian (#3)
Re: plpython improvements

Hi,

Mon, 17 Apr 2006 19:20:38 +0300, Bruce Momjian <pgman@candle.pha.pa.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

Attachments:

plpython.patchtext/x-patch; name=plpython.patchDownload+230-9
#5Bruce Momjian
bruce@momjian.us
In reply to: Sven Suursoho (#4)
Re: plpython improvements

Patch applied. Thanks.

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

Sven Suursoho wrote:

Hi,

Mon, 17 Apr 2006 19:20:38 +0300, Bruce Momjian <pgman@candle.pha.pa.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. +

#6Bruce Momjian
bruce@momjian.us
In reply to: Bruce Momjian (#5)
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@candle.pha.pa.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. +

Attachments:

/pgpatches/plpythontext/plainDownload+237-25
#7Hannu Krosing
hannu@tm.ee
In reply to: Bruce Momjian (#6)
Re: plpython improvements

Ü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

#8Bruce Momjian
bruce@momjian.us
In reply to: Hannu Krosing (#7)
Re: plpython improvements

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&amp;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. +

#9Sven Suursoho
sven@spam.pri.ee
In reply to: Bruce Momjian (#6)
Re: plpython improvements

Hi,

Thu, 27 Apr 2006 17:17:36 +0300, Bruce Momjian <pgman@candle.pha.pa.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

Attachments:

plpython.patchtext/x-patch; name=plpython.patchDownload+670-78
#10Tom Lane
tgl@sss.pgh.pa.us
In reply to: Sven Suursoho (#9)
Re: plpython improvements

"Sven Suursoho" <sven@spam.pri.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

#11Sven Suursoho
sven@spam.pri.ee
In reply to: Tom Lane (#10)
Re: plpython improvements

Sun, 30 Apr 2006 19:14:28 +0300, Tom Lane <tgl@sss.pgh.pa.us>:

"Sven Suursoho" <sven@spam.pri.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

#12Bruce Momjian
bruce@momjian.us
In reply to: Sven Suursoho (#11)
Re: plpython improvements

Sven Suursoho wrote:

Sun, 30 Apr 2006 19:14:28 +0300, Tom Lane <tgl@sss.pgh.pa.us>:

"Sven Suursoho" <sven@spam.pri.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. +

#13Sven Suursoho
sven@spam.pri.ee
In reply to: Bruce Momjian (#12)
Re: plpython improvements

Sun, 30 Apr 2006 20:48:48 +0300, Bruce Momjian <pgman@candle.pha.pa.us>:

Sun, 30 Apr 2006 19:14:28 +0300, Tom Lane <tgl@sss.pgh.pa.us>:

"Sven Suursoho" <sven@spam.pri.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

#14Tom Lane
tgl@sss.pgh.pa.us
In reply to: Sven Suursoho (#13)
Re: plpython improvements

"Sven Suursoho" <sven@spam.pri.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

#15Sven Suursoho
sven@spam.pri.ee
In reply to: Tom Lane (#14)
Re: plpython improvements

Sun, 30 Apr 2006 21:43:03 +0300, Tom Lane <tgl@sss.pgh.pa.us>:

"Sven Suursoho" <sven@spam.pri.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

#16Hannu Krosing
hannu@tm.ee
In reply to: Tom Lane (#14)
Re: plpython improvements

Ühel kenal päeval, P, 2006-04-30 kell 14:43, kirjutas Tom Lane:

"Sven Suursoho" <sven@spam.pri.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

#17Sven Suursoho
sven@spam.pri.ee
In reply to: Tom Lane (#10)
Re: plpython improvements

Hi,

Sun, 30 Apr 2006 19:14:28 +0300, Tom Lane <tgl@sss.pgh.pa.us>:

"Sven Suursoho" <sven@spam.pri.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

Attachments:

plpython.patchtext/x-patch; name=plpython.patchDownload+656-79
#18Hannu Krosing
hannu@tm.ee
In reply to: Sven Suursoho (#17)
Re: plpython improvements

Ü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@sss.pgh.pa.us>:

"Sven Suursoho" <sven@spam.pri.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&amp;aid=1257960&amp;group_id=5470&amp;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

#19Joshua D. Drake
jd@commandprompt.com
In reply to: Hannu Krosing (#18)
Re: plpython improvements

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/

#20Sven Suursoho
sven@spam.pri.ee
In reply to: Joshua D. Drake (#19)
Re: plpython improvements

Fri, 05 May 2006 19:20:55 +0300, Joshua D. Drake <jd@commandprompt.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

#21Joshua D. Drake
jd@commandprompt.com
In reply to: Sven Suursoho (#20)
#22Hannu Krosing
hannu@tm.ee
In reply to: Joshua D. Drake (#19)
#23Bruce Momjian
bruce@momjian.us
In reply to: Sven Suursoho (#20)
#24Sven Suursoho
sven@spam.pri.ee
In reply to: Bruce Momjian (#23)
#25Sven Suursoho
sven@spam.pri.ee
In reply to: Sven Suursoho (#24)
#26Bruce Momjian
bruce@momjian.us
In reply to: Sven Suursoho (#24)
#27Sven Suursoho
sven@spam.pri.ee
In reply to: Bruce Momjian (#26)
#28Bruce Momjian
bruce@momjian.us
In reply to: Hannu Krosing (#18)
#29Sven Suursoho
sven@spam.pri.ee
In reply to: Bruce Momjian (#28)
#30Tom Lane
tgl@sss.pgh.pa.us
In reply to: Sven Suursoho (#29)
#31Sven Suursoho
public@spam.pri.ee
In reply to: Tom Lane (#30)
#32Sven Suursoho
pg@spam.pri.ee
In reply to: Sven Suursoho (#31)
#33Bruce Momjian
bruce@momjian.us
In reply to: Sven Suursoho (#32)
#34Sven Suursoho
pg@spam.pri.ee
In reply to: Bruce Momjian (#33)
#35Bruce Momjian
bruce@momjian.us
In reply to: Sven Suursoho (#34)