Re: [HACKERS] pgsql: Fix for plpython functions; return true/false for boolean,

Log Message:
-----------
Fix for plpython functions; return true/false for boolean,
rather than 1/0. This helps when creating trigger functions that output
SQL.

Guido Goldstein

Modified Files:
--------------
pgsql/src/pl/plpython:
plpython.c (r1.90 -> r1.91)
(http://developer.postgresql.org/cvsweb.cgi/pgsql/src/pl/plpython/plpython.c.diff?r1=1.90&r2=1.91)

momjian(at)postgresql(dot)org (Bruce Momjian) writes:
> Fix for plpython functions; return true/false for boolean,

This patch has broken a majority of the buildfarm.

regards, tom lane

Tom Lane wrote:
> momjian(at)postgresql(dot)org (Bruce Momjian) writes:
> > Fix for plpython functions; return true/false for boolean,
>
> This patch has broken a majority of the buildfarm.

Yea, reverted with comment added.

--
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!

Sorry for the late reply.

On Thu, 25 Jan 2007 01:52:32 -0500
Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> momjian(at)postgresql(dot)org (Bruce Momjian) writes:
>> Fix for plpython functions; return true/false for boolean,
>
> This patch has broken a majority of the buildfarm.
>

Is it possible to tell me which python versions you want to
support?

Just as a hint: 2.5 is the current stable version.

Cheers
Guido

Guido Goldstein wrote:
> Is it possible to tell me which python versions you want to
> support?

The issue isn't so much which versions we want to support. There is
certainly some flexibility with that. But when a patch breaks the
buildfarm a) unannounced and b) without any apparent feature gain, then
people get annoyed.

That said, we certainly try to support a few more versions of Python
than just the last one, but I'm not sure anyone knows which ones
exactly. As a data point: Quite probably, Python 2.5 does *not* work
with anything <= 8.1, so it would be nice if we could give the Python
2.4 users the option of not having to upgrade to Python 2.5 at the same
time as upgrading to PostgreSQL 8.2. This doesn't really govern your
8.3 patch, however.

--
Peter Eisentraut
http://developer.postgresql.org/~petere/

Peter Eisentraut wrote:
> Guido Goldstein wrote:
>> Is it possible to tell me which python versions you want to
>> support?
>
> The issue isn't so much which versions we want to support. There is
> certainly some flexibility with that. But when a patch breaks the
> buildfarm a) unannounced and b) without any apparent feature gain, then
> people get annoyed.

If this breaks the buildfarm it's not my failure.
Except you can tell me what I've got to do with the
buildfarm.

If you mean that plpython didn't compile, fine; simply tell
the people what version they should consider when sending
in patches.

I've checked the patch with postgres 8.1.3 and 8.2.1
with python 2.4 and 2.5 on intel 32 bit and amd 64 bit
systems; all systems running linux.

*And* it's not a feature patch but a bug-fixing one!
Python is a language with strong typing, so silently
converting a datatype is a bug -- not a feature.
Btw, you'll lose the type information of boolean columns in
trigger functions (NEW and OLD dicts, no explicit parameters),
which does cause problems.

> That said, we certainly try to support a few more versions of Python
[...]

If you want to support python 2.3 use the attached patch, which also
works for the newer python versions.
The Python 2.3 branch is the oldest _officially_ supported python version.

Anyway, to circumvent the above mentiond point a) I herewith anncounce
that the included patch might break the buildfarm.

Cheers
Guido

Guido Goldstein wrote:
> Is it possible to tell me which python versions you want to
> support?
>
>
>

There are still products shipping with 2.3 (e.g. RHEL4). I'd be
surprised if we need to go back before that.

cheers

andrew

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> Guido Goldstein wrote:
>> Is it possible to tell me which python versions you want to
>> support?

> There are still products shipping with 2.3 (e.g. RHEL4). I'd be
> surprised if we need to go back before that.

As far as Red Hat is concerned, we won't be trying to get PG 8.3 and up
to run on anything older than RHEL4, so python 2.3 is old enough. Not
sure how the release timing has worked out for other distros ... but the
presence of python 2.3 in the buildfarm says to me that it's still
fairly popular.

[ digs a bit more... ] Actually, it looks like Fedora Core 1 shipped
with python 2.2.3, which means that's what buildfarm member "thrush"
is running. So you probably don't want to break 2.2 either, at least
not for a basically cosmetic patch. I don't say that we'd reject a
patch that breaks 2.2 compatibility, but you'd need to put forth a
sufficient justification.

regards, tom lane

On Jan 30, 2007, at 2:43 AM, Guido Goldstein wrote:
> Is it possible to tell me which python versions you want to
> support?
>
> Just as a hint: 2.5 is the current stable version.

I support a lot of python on several platforms. For broad
compatibility with pre-installed Python versions on recent OS
versions, Python 2.3 support is essentially mandatory and there are
few good reasons to not support it. I occasionally see Python 2.2 on
really old systems by default, but it takes significantly more effort
to support versions that old; the solution in my case is to upgrade
Python to 2.3 or 2.4.

Python 2.5 may be the current "stable" version, but vanilla source
builds segfault on some Python code that runs fine in 2.3 and 2.4,
strongly suggesting that it is not mature enough that I would put it
anywhere near anything important (like a database).

J. Andrew Rogers
jrogers(at)neopolitan(dot)com

Ühel kenal päeval, T, 2007-01-30 kell 14:52, kirjutas Guido Goldstein:

> I've checked the patch with postgres 8.1.3 and 8.2.1
> with python 2.4 and 2.5 on intel 32 bit and amd 64 bit
> systems; all systems running linux.
>
> *And* it's not a feature patch but a bug-fixing one!
> Python is a language with strong typing, so silently
> converting a datatype is a bug -- not a feature.

Python is not that strongly typed. More it is a protocol based language,
meaning that you should not relay on "type" of any variable, but rather
see if it does what you want - so any type supporting iteration can be
used if "for" and any thing not None, 0 or empty sequence/dict is
considered to be TRUE

True and False are actually 1 and 0 with different spelling ;)

>>> True+2
3
>>> 1/False
Traceback (most recent call last):
File "<stdin>", line 1, in ?
ZeroDivisionError: integer division or modulo by zero

> Btw, you'll lose the type information of boolean columns in
> trigger functions (NEW and OLD dicts, no explicit parameters),
> which does cause problems.
>
> > That said, we certainly try to support a few more versions of Python
> [...]
>
> If you want to support python 2.3 use the attached patch, which also
> works for the newer python versions.
> The Python 2.3 branch is the oldest _officially_ supported python version.

Officially by who ?

2.3 was the first version to introduce bool as a subtype of int, in
2.2.3 True and False were introduced as two variables pointing to
integers 1 and 0.

So to make your patch ok on all python versions, just make it
conditional on python version being 2.3 or bigger, and return int for
pre-2.3.

> Anyway, to circumvent the above mentiond point a) I herewith anncounce
> that the included patch might break the buildfarm.

:)

> Cheers
> Guido
>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: don't forget to increase your free space map settings
--
----------------
Hannu Krosing
Database Architect
Skype Technologies OÜ
Akadeemia tee 21 F, Tallinn, 12618, Estonia

Skype me: callto:hkrosing
Get Skype for free: http://www.skype.com

Hannu Krosing wrote:
> Officially by who ?
>
> 2.3 was the first version to introduce bool as a subtype of int, in
> 2.2.3 True and False were introduced as two variables pointing to
> integers 1 and 0.
>
> So to make your patch ok on all python versions, just make it
> conditional on python version being 2.3 or bigger, and return int for
> pre-2.3.

I thought about suggesting that, but do we want plpython to have
different result behavior based on the version of python used? I didn't
think so.

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

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

Bruce Momjian wrote:
> Hannu Krosing wrote:
> > Officially by who ?
> >
> > 2.3 was the first version to introduce bool as a subtype of int, in
> > 2.2.3 True and False were introduced as two variables pointing to
> > integers 1 and 0.
> >
> > So to make your patch ok on all python versions, just make it
> > conditional on python version being 2.3 or bigger, and return int for
> > pre-2.3.
>
> I thought about suggesting that, but do we want plpython to have
> different result behavior based on the version of python used? I didn't
> think so.

The alternative would be, what, including the whole python source in our
distribution? Because the Python guys themselves changed the behavior
depending on the version.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Bruce Momjian schrieb:
> Hannu Krosing wrote:
>> Officially by who ?
>>
>> 2.3 was the first version to introduce bool as a subtype of int, in
>> 2.2.3 True and False were introduced as two variables pointing to
>> integers 1 and 0.
>>
>> So to make your patch ok on all python versions, just make it
>> conditional on python version being 2.3 or bigger, and return int for
>> pre-2.3.
>
> I thought about suggesting that, but do we want plpython to have
> different result behavior based on the version of python used? I didn't
> think so.

Why not? Python2.2 is rarely in use anymore and users of this would get
the same behavior. Users of python2.3 and up would get the additionally
cleaned boolean interface - also users which go the from __future__
import ... way. Thats how python works and develops forth and we should
not work against that from postgres side.

So I'm indeed +1 for conditional approach.

Regards
Tino

Tino Wildenhain wrote:
> Bruce Momjian schrieb:
> > Hannu Krosing wrote:
> >> Officially by who ?
> >>
> >> 2.3 was the first version to introduce bool as a subtype of int, in
> >> 2.2.3 True and False were introduced as two variables pointing to
> >> integers 1 and 0.
> >>
> >> So to make your patch ok on all python versions, just make it
> >> conditional on python version being 2.3 or bigger, and return int for
> >> pre-2.3.
> >
> > I thought about suggesting that, but do we want plpython to have
> > different result behavior based on the version of python used? I didn't
> > think so.
>
> Why not? Python2.2 is rarely in use anymore and users of this would get
> the same behavior. Users of python2.3 and up would get the additionally
> cleaned boolean interface - also users which go the from __future__
> import ... way. Thats how python works and develops forth and we should
> not work against that from postgres side.
>
> So I'm indeed +1 for conditional approach.

Fine if people think that is OK. Please submit a patch that is
conditional on the python version.

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

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

Tino Wildenhain <tino(at)wildenhain(dot)de> writes:
> Bruce Momjian schrieb:
>> I thought about suggesting that, but do we want plpython to have
>> different result behavior based on the version of python used? I didn't
>> think so.

> Why not?

Indeed --- the underlying language changed, so I should think that
python users would *expect* different behavior. +1 on a conditional
patch (see PY_VERSION_HEX...)

regards, tom lane

I am still waiting for a plpython patch that has Python version
checking.

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

Guido Goldstein wrote:
> Peter Eisentraut wrote:
> > Guido Goldstein wrote:
> >> Is it possible to tell me which python versions you want to
> >> support?
> >
> > The issue isn't so much which versions we want to support. There is
> > certainly some flexibility with that. But when a patch breaks the
> > buildfarm a) unannounced and b) without any apparent feature gain, then
> > people get annoyed.
>
> If this breaks the buildfarm it's not my failure.
> Except you can tell me what I've got to do with the
> buildfarm.
>
> If you mean that plpython didn't compile, fine; simply tell
> the people what version they should consider when sending
> in patches.
>
> I've checked the patch with postgres 8.1.3 and 8.2.1
> with python 2.4 and 2.5 on intel 32 bit and amd 64 bit
> systems; all systems running linux.
>
> *And* it's not a feature patch but a bug-fixing one!
> Python is a language with strong typing, so silently
> converting a datatype is a bug -- not a feature.
> Btw, you'll lose the type information of boolean columns in
> trigger functions (NEW and OLD dicts, no explicit parameters),
> which does cause problems.
>
> > That said, we certainly try to support a few more versions of Python
> [...]
>
> If you want to support python 2.3 use the attached patch, which also
> works for the newer python versions.
> The Python 2.3 branch is the oldest _officially_ supported python version.
>
> Anyway, to circumvent the above mentiond point a) I herewith anncounce
> that the included patch might break the buildfarm.
>
> Cheers
> Guido
>

>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: don't forget to increase your free space map settings

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

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

Added to TODO:

o Allow PL/Python to return boolean rather than 1/0

http://archives.postgresql.org/pgsql-patches/2007-01/msg00596$

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

Guido Goldstein wrote:
> Peter Eisentraut wrote:
> > Guido Goldstein wrote:
> >> Is it possible to tell me which python versions you want to
> >> support?
> >
> > The issue isn't so much which versions we want to support. There is
> > certainly some flexibility with that. But when a patch breaks the
> > buildfarm a) unannounced and b) without any apparent feature gain, then
> > people get annoyed.
>
> If this breaks the buildfarm it's not my failure.
> Except you can tell me what I've got to do with the
> buildfarm.
>
> If you mean that plpython didn't compile, fine; simply tell
> the people what version they should consider when sending
> in patches.
>
> I've checked the patch with postgres 8.1.3 and 8.2.1
> with python 2.4 and 2.5 on intel 32 bit and amd 64 bit
> systems; all systems running linux.
>
> *And* it's not a feature patch but a bug-fixing one!
> Python is a language with strong typing, so silently
> converting a datatype is a bug -- not a feature.
> Btw, you'll lose the type information of boolean columns in
> trigger functions (NEW and OLD dicts, no explicit parameters),
> which does cause problems.
>
> > That said, we certainly try to support a few more versions of Python
> [...]
>
> If you want to support python 2.3 use the attached patch, which also
> works for the newer python versions.
> The Python 2.3 branch is the oldest _officially_ supported python version.
>
> Anyway, to circumvent the above mentiond point a) I herewith anncounce
> that the included patch might break the buildfarm.
>
> Cheers
> Guido
>

>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: don't forget to increase your free space map settings

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

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