pgsql: Fix for plpython functions; return true/false for boolean,

Started by Nonamealmost 19 years ago17 messages
#1Noname
momjian@postgresql.org

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)

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Noname (#1)
Re: pgsql: Fix for plpython functions; return true/false for boolean,

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

#3Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#2)
Re: pgsql: Fix for plpython functions; return

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. +

#4Guido Goldstein
pgpatches@a-nugget.org
In reply to: Noname (#1)
Re: pgsql: Fix for plpython functions; return true/false for boolean,

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

#5Peter Eisentraut
peter_e@gmx.net
In reply to: Guido Goldstein (#4)
Re: pgsql: Fix for plpython functions; return true/false for boolean,

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/

#6Guido Goldstein
guido.goldstein@a-nugget.org
In reply to: Peter Eisentraut (#5)
1 attachment(s)
Re: [HACKERS] pgsql: Fix for plpython functions; return true/false for boolean,

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

Attachments:

pg-plpython.difftext/x-patch; name=pg-plpython.diffDownload
--- 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 *
#7Andrew Dunstan
andrew@dunslane.net
In reply to: Guido Goldstein (#4)
Re: pgsql: Fix for plpython functions; return true/false for boolean,

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

#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#7)
Re: pgsql: Fix for plpython functions; return true/false for boolean,

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

#9J. Andrew Rogers
jrogers@neopolitan.com
In reply to: Guido Goldstein (#4)
Re: pgsql: Fix for plpython functions; return true/false for boolean,

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

#10Hannu Krosing
hannu@skype.net
In reply to: Guido Goldstein (#6)
Re: [HACKERS] pgsql: Fix for plpython functions; return true/false for boolean,

Ü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

#11Bruce Momjian
bruce@momjian.us
In reply to: Hannu Krosing (#10)
Re: [HACKERS] pgsql: Fix for plpython functions; return true/false for boolean,

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. +

#12Alvaro Herrera
alvherre@commandprompt.com
In reply to: Bruce Momjian (#11)
Re: [HACKERS] pgsql: Fix for plpython functions; return true/false for boolean,

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.

#13Tino Wildenhain
tino@wildenhain.de
In reply to: Bruce Momjian (#11)
Re: [HACKERS] pgsql: Fix for plpython functions; return true/false for boolean,

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

#14Bruce Momjian
bruce@momjian.us
In reply to: Tino Wildenhain (#13)
Re: [HACKERS] pgsql: Fix for plpython functions; return true/false for boolean,

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. +

#15Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tino Wildenhain (#13)
Re: [HACKERS] pgsql: Fix for plpython functions; return true/false for boolean,

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

#16Bruce Momjian
bruce@momjian.us
In reply to: Guido Goldstein (#6)
Re: pgsql: Fix for plpython functions; return true/false for boolean,

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. +

#17Bruce Momjian
bruce@momjian.us
In reply to: Guido Goldstein (#6)
Re: [HACKERS] pgsql: Fix for plpython functions; return true/false for boolean,

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. +