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.
diff -U3 
/home/nm/src/pg/vp/3stub/src/pl/plpython/expected/python3/plpython_stub.out 
/home/nm/src/pg/vp/3stub/src/pl/plpython/results/python3/plpython_stub.out
--- /home/nm/src/pg/vp/3stub/src/pl/plpython/expected/python3/plpython_stub.out 
2020-05-28 01:01:35.445842589 -0700
+++ /home/nm/src/pg/vp/3stub/src/pl/plpython/results/python3/plpython_stub.out  
2020-05-28 01:01:37.765861474 -0700
@@ -50,22 +50,33 @@
 
 call convert_python3_all();
 NOTICE:  converting function convert1()
-NOTICE:  converting function convert2()
+ERROR:  subprocess.CalledProcessError: Command '2to3  --no-diffs -w 
/tmp/tmpvmbr920w/temp.py' returned non-zero exit status 127.
+CONTEXT:  Traceback (most recent call last):
+  PL/Python function "convert_python3_all", line 42, in <module>
+    subprocess.check_call(tool + " " + options + " --no-diffs -w " + 
tmpdirname + "/temp.py", shell=True)
+  PL/Python function "convert_python3_all", line 310, in check_call
+PL/Python procedure "convert_python3_all"
 \sf convert1()
 CREATE OR REPLACE FUNCTION public.convert1()
  RETURNS integer
- LANGUAGE plpython3u
+ LANGUAGE plpython2u
  IMMUTABLE
-AS $function$return 123$function$
+AS $function$return 123l$function$
 \sf convert2()
 CREATE OR REPLACE FUNCTION public.convert2()
  RETURNS integer
- LANGUAGE plpython3u
+ LANGUAGE plpythonu
  IMMUTABLE
-AS $function$return 123$function$
+AS $function$return 123l$function$
 -- clean up
 DROP EXTENSION
 plpython2u;
+ERROR:  cannot drop extension plpython2u because other objects depend on it
+DETAIL:  function convert1() depends on language plpython2u
+HINT:  Use DROP ... CASCADE to drop the dependent objects too.
 DROP EXTENSION
 plpythonu;
+ERROR:  cannot drop extension plpythonu because other objects depend on it
+DETAIL:  function convert2() depends on language plpythonu
+HINT:  Use DROP ... CASCADE to drop the dependent objects too.
 DROP EXTENSION convert_python3;

Reply via email to