Re: Resolving the python 2 -> python 3 mess

On Wed, Feb 19, 2020 at 08:42:36PM +0100, Peter Eisentraut wrote:
> I think there should just
> be an option "plpython is: {2|3|don't build it at all}".  Then packagers can
> match this to what their plan for /usr/bin/python* is -- which appears to be
> different everywhere.

Today, we do not give packagers this sort of discretion over SQL-level
behavior.  We formerly had --disable-integer-datetimes, but I view that as the
lesser of two evils, the greater of which would have been to force dump/reload
of terabyte-scale clusters.  We should continue to follow today's principle,
which entails not inviting packagers to align the nature of "LANGUAGE
plpythonu" with the nature of /usr/bin/python.  Users wouldn't benefit from
alignment.  Moreover, it has long been conventional for the host to be a VM
dedicated to PostgreSQL, in which case users of the DBMS aren't users of its
host.  I don't have an opinion on which version "LANGUAGE plpythonu" should
denote in PostgreSQL v13, but the version should be constant across v13
configurations.  (If some builds make it unavailable or make it an error-only
stub, that is no problem.)

I reviewed all code:

On Thu, Feb 27, 2020 at 04:11:05PM -0500, Tom Lane wrote:
> --- a/configure.in
> +++ b/configure.in
> @@ -766,6 +766,9 @@ PGAC_ARG_BOOL(with, python, no, [build Python modules (PL/Python)])
>  AC_MSG_RESULT([$with_python])
>  AC_SUBST(with_python)
>  
> +PGAC_ARG_BOOL(with, python2-stub, no, [build Python 2 compatibility stub])
> +AC_SUBST(with_python2_stub)
> +
>  #
>  # GSSAPI
>  #
> @@ -1042,6 +1045,12 @@ fi
>  if test "$with_python" = yes; then
>    PGAC_PATH_PYTHON
>    PGAC_CHECK_PYTHON_EMBED_SETUP
> +  # Disable building Python 2 stub if primary version isn't Python 3
> +  if test "$python_majorversion" -lt 3; then
> +    with_python2_stub=no
> +  fi

Standard PostgreSQL practice would be to AC_MSG_ERROR in response to the
infeasible option, not to ignore the option.

> --- /dev/null
> +++ b/src/pl/plpython/sql/plpython_stub.sql

> +call convert_python3_all();

Building with PYTHON=python3 --with-python2-stub, this fails on my RHEL 7.8
installation.  I had installed python34-tools, which provides /usr/bin/2to3-3.
I had not installed python-tools (v2.7.5), which provides /usr/bin/2to3.
Making a 2to3 -> 2to3-3 symlink let the test pass.  Requiring such a step to
pass tests may or may not be fine; what do you think?  (I am attaching
regression.diffs; to my knowledge, it's not interesting.)

> --- a/src/tools/msvc/config_default.pl
> +++ b/src/tools/msvc/config_default.pl
> @@ -16,6 +16,7 @@ our $config = {
>      tcl       => undef,    # --with-tcl=<path>
>      perl      => undef,    # --with-perl=<path>
>      python    => undef,    # --with-python=<path>
> +    python2_stub => 1,     # --with-python2-stub (ignored unless Python is v3)

This default should not depend on whether one uses the MSVC build system or
uses the GNU make build system.

> --- /dev/null
> +++ b/src/pl/plpython/convert_python3--1.0.sql

> +create procedure convert_python3_all(tool text default '2to3',
> +                                     options text default '')
> +language plpython3u as $$
> +import re, subprocess, tempfile
> +
> +# pattern to extract just the function header from pg_get_functiondef result
> +aspat = re.compile("^(.*?\nAS )", re.DOTALL)

This fails on:

create function convert1("
AS " int) returns int
AS $$return 123l$$
language
plpython2u
immutable;

That's not up to project standard, but I'm proceeding to ignore this since the
subject is an untrusted language and ~nobody uses such argument names.