Thread: Re: [COMMITTERS] pgsql: Fix for plpython functions; return true/false for boolean,
Re: [COMMITTERS] pgsql: Fix for plpython functions; return true/false for boolean,
From
Tom Lane
Date:
momjian@postgresql.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@postgresql.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@momjian.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@sss.pgh.pa.us> wrote: > momjian@postgresql.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/
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@dunslane.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@neopolitan.com
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
--- postgresql-8.2.1.orig/src/pl/plpython/plpython.c 2006-11-21 22:51:05.000000000 +0100
+++ postgresql-8.2.1/src/pl/plpython/plpython.c 2007-01-17 18:06:58.185497734 +0100
@@ -1580,8 +1580,8 @@
PLyBool_FromString(const char *src)
{
if (src[0] == 't')
- return PyInt_FromLong(1);
- return PyInt_FromLong(0);
+ return PyBool_FromLong(1);
+ return PyBool_FromLong(0);
}
static PyObject *
Ü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@momjian.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@momjian.us EnterpriseDB http://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. +
Tino Wildenhain <tino@wildenhain.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@momjian.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@momjian.us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +