Thread: 7.3.3 - plpython & trigger problem

7.3.3 - plpython & trigger problem

From
Arthur Ward
Date:
I'm running "PostgreSQL 7.3.3 on i686-pc-linux-gnu, compiled by
GCC gcc (GCC) 3.2.2", and seeing a problem in trigger functions
written in plpython. This also happens in 7.3.2; I upgraded this
morning to see if the problem was already fixed. For fields of type
char, when a plpython trigger is called, the char fields in the
TD["new"] or TD["old"] maps show as 0 or 1 instead of their actual
value. As far as I've seen right now, it only affects char. The
varchar and text types seem ok, but I have not done any
comprehensive testing of the numerics and other types.

Here's a test script to show the problem:
CREATE TABLE test (
    broken CHAR(4),
    works VARCHAR(4),
    alsoworks TEXT
);
CREATE OR REPLACE FUNCTION pytest() RETURNS TRIGGER
AS '
plpy.info ("TD=%s" % TD)
return None
' LANGUAGE 'plpython';
CREATE TRIGGER test_trig BEFORE INSERT OR UPDATE OR
DELETE ON test
  FOR EACH ROW EXECUTE PROCEDURE pytest();
INSERT INTO test VALUES ('test', 'test', 'test');
UPDATE test SET broken='1234', works='1234', alsoworks='1234';
DELETE FROM test;

And here's the output from the last three statements on my system:
award=> INSERT INTO test VALUES ('test', 'test', 'test');
INFO:  ("TD={'relid': '3928912', 'old': None, 'name': 'test_trig', 'level':
'ROW', 'new': {'alsoworks': 'test', 'broken': 1, 'works': 'test'}, 'args':
None, 'when': 'BEFORE', 'event': 'INSERT'}",)
INSERT 3928918 1
award=> UPDATE test SET broken='1234', works='1234',
alsoworks='1234';
INFO:  ("TD={'relid': '3928912', 'old': {'alsoworks': 'test', 'broken': 1,
'works': 'test'}, 'name': 'test_trig', 'level': 'ROW', 'new': {'alsoworks':
'1234', 'broken': 0, 'works': '1234'}, 'args': None, 'when': 'BEFORE',
'event': 'UPDATE'}",)
UPDATE 1
award=> DELETE FROM test;
INFO:  ("TD={'relid': '3928912', 'old': {'alsoworks': '1234', 'broken': 0,
'works': '1234'}, 'name': 'test_trig', 'level': 'ROW', 'new': None, 'args':
None, 'when': 'BEFORE', 'event': 'DELETE'}",)
DELETE 1


--
Arthur Ward

Re: 7.3.3 - plpython & trigger problem

From
Tom Lane
Date:
Arthur Ward <ward005@bama.ua.edu> writes:
> I'm running "PostgreSQL 7.3.3 on i686-pc-linux-gnu, compiled by
> GCC gcc (GCC) 3.2.2", and seeing a problem in trigger functions
> written in plpython.

There is a known bug with applying the same plpython trigger function to
multiple tables that have different rowtypes --- the rowtype info gets
cached for the first table the function is used with in a session, and
then inappropriately reused with the other tables.  Does this seem to
describe your observations?

Someone had promised to supply a fix, but I haven't seen it yet.
In the meantime the only known workaround is to create multiple
copies of the plpython function, one for each trigger.

            regards, tom lane

Re: 7.3.3 - plpython & trigger problem

From
Arthur Ward
Date:
> There is a known bug with applying the same plpython trigger function to
> multiple tables that have different rowtypes --- the rowtype info gets
> cached for the first table the function is used with in a session, and
> then inappropriately reused with the other tables.  Does this seem to
> describe your observations?

No, it does not match what I'm seeing. To check, I dropped my test
table and function, exited psql, started psql, and ran the script I
originally posted. I still get the same results. If I had to guess, this
seems more like a Postgres to plpython conversion problem than a
caching problem. Another thing that seems weird about it is that the
value is sometimes 1 and sometimes 0. In my test case, it's 1 when
the value should be 'test', and 0 when '1234'.

Had I not noticed plpython's problem with char fields, I would have
already run smack into the caching problem you described. :-(

--
Arthur Ward

Re: 7.3.3 - plpython & trigger problem

From
Tom Lane
Date:
Arthur Ward <ward005@bama.ua.edu> writes:
> No, it does not match what I'm seeing. To check, I dropped my test
> table and function, exited psql, started psql, and ran the script I
> originally posted. I still get the same results. If I had to guess, this
> seems more like a Postgres to plpython conversion problem than a
> caching problem.

Bingo.  The conversion-method-selection code is not only broken (for
several datatypes besides this one), but quite wrongheaded (testing on
the name of the datatype is the wrong approach).  I've applied the
attached patch; please confirm it works for you.

            regards, tom lane


*** src/pl/plpython/plpython.c.orig    Thu Feb 13 18:06:19 2003
--- src/pl/plpython/plpython.c    Wed Jun 11 14:31:23 2003
***************
*** 244,251 ****
  static void PLy_typeinfo_dealloc(PLyTypeInfo *);
  static void PLy_output_datum_func(PLyTypeInfo *, Form_pg_type);
  static void PLy_output_datum_func2(PLyObToDatum *, Form_pg_type);
! static void PLy_input_datum_func(PLyTypeInfo *, Form_pg_type);
! static void PLy_input_datum_func2(PLyDatumToOb *, Form_pg_type);
  static void PLy_output_tuple_funcs(PLyTypeInfo *, TupleDesc);
  static void PLy_input_tuple_funcs(PLyTypeInfo *, TupleDesc);

--- 244,251 ----
  static void PLy_typeinfo_dealloc(PLyTypeInfo *);
  static void PLy_output_datum_func(PLyTypeInfo *, Form_pg_type);
  static void PLy_output_datum_func2(PLyObToDatum *, Form_pg_type);
! static void PLy_input_datum_func(PLyTypeInfo *, Oid, Form_pg_type);
! static void PLy_input_datum_func2(PLyDatumToOb *, Oid, Form_pg_type);
  static void PLy_output_tuple_funcs(PLyTypeInfo *, TupleDesc);
  static void PLy_input_tuple_funcs(PLyTypeInfo *, TupleDesc);

***************
*** 1152,1158 ****
          argTypeStruct = (Form_pg_type) GETSTRUCT(argTypeTup);

          if (argTypeStruct->typrelid == InvalidOid)
!             PLy_input_datum_func(&(proc->args[i]), argTypeStruct);
          else
          {
              TupleTableSlot *slot = (TupleTableSlot *) fcinfo->arg[i];
--- 1152,1160 ----
          argTypeStruct = (Form_pg_type) GETSTRUCT(argTypeTup);

          if (argTypeStruct->typrelid == InvalidOid)
!             PLy_input_datum_func(&(proc->args[i]),
!                                  procStruct->proargtypes[i],
!                                  argTypeStruct);
          else
          {
              TupleTableSlot *slot = (TupleTableSlot *) fcinfo->arg[i];
***************
*** 1373,1379 ****

          typeStruct = (Form_pg_type) GETSTRUCT(typeTup);

!         PLy_input_datum_func2(&(arg->in.r.atts[i]), typeStruct);

          ReleaseSysCache(typeTup);
      }
--- 1375,1383 ----

          typeStruct = (Form_pg_type) GETSTRUCT(typeTup);

!         PLy_input_datum_func2(&(arg->in.r.atts[i]),
!                               desc->attrs[i]->atttypid,
!                               typeStruct);

          ReleaseSysCache(typeTup);
      }
***************
*** 1439,1523 ****
  }

  void
! PLy_input_datum_func(PLyTypeInfo * arg, Form_pg_type typeStruct)
  {
      enter();

      if (arg->is_rel == 1)
          elog(FATAL, "plpython: PLyTypeInfo struct is initialized for Tuple");
      arg->is_rel = 0;
!     PLy_input_datum_func2(&(arg->in.d), typeStruct);
  }

  void
! PLy_input_datum_func2(PLyDatumToOb * arg, Form_pg_type typeStruct)
  {
!     char       *type;
!
      perm_fmgr_info(typeStruct->typoutput, &arg->typfunc);
      arg->typelem = typeStruct->typelem;
      arg->typbyval = typeStruct->typbyval;

!     /*
!      * hmmm, wierd.  means this arg will always be converted to a python
!      * None
!      */
!     if (!OidIsValid(typeStruct->typoutput))
!     {
!         elog(ERROR, "plpython: (FIXME) typeStruct->typoutput is invalid");
!
!         arg->func = NULL;
!         return;
!     }
!
!     type = NameStr(typeStruct->typname);
!     switch (type[0])
      {
!         case 'b':
!             {
!                 if (strcasecmp("bool", type))
!                 {
!                     arg->func = PLyBool_FromString;
!                     return;
!                 }
!                 break;
!             }
!         case 'f':
!             {
!                 if ((strncasecmp("float", type, 5) == 0) &&
!                     ((type[5] == '8') || (type[5] == '4')))
!                 {
!                     arg->func = PLyFloat_FromString;
!                     return;
!                 }
!                 break;
!             }
!         case 'i':
!             {
!                 if ((strncasecmp("int", type, 3) == 0) &&
!                     ((type[3] == '4') || (type[3] == '2')) &&
!                     (type[4] == '\0'))
!                 {
!                     arg->func = PLyInt_FromString;
!                     return;
!                 }
!                 else if (strcasecmp("int8", type) == 0)
!                     arg->func = PLyLong_FromString;
!                 break;
!             }
!         case 'n':
!             {
!                 if (strcasecmp("numeric", type) == 0)
!                 {
!                     arg->func = PLyFloat_FromString;
!                     return;
!                 }
!                 break;
!             }
          default:
              break;
      }
-     arg->func = PLyString_FromString;
  }

  void
--- 1443,1488 ----
  }

  void
! PLy_input_datum_func(PLyTypeInfo * arg, Oid typeOid, Form_pg_type typeStruct)
  {
      enter();

      if (arg->is_rel == 1)
          elog(FATAL, "plpython: PLyTypeInfo struct is initialized for Tuple");
      arg->is_rel = 0;
!     PLy_input_datum_func2(&(arg->in.d), typeOid, typeStruct);
  }

  void
! PLy_input_datum_func2(PLyDatumToOb * arg, Oid typeOid, Form_pg_type typeStruct)
  {
!     /* Get the type's conversion information */
      perm_fmgr_info(typeStruct->typoutput, &arg->typfunc);
      arg->typelem = typeStruct->typelem;
      arg->typbyval = typeStruct->typbyval;

!     /* Determine which kind of Python object we will convert to */
!     switch (typeOid)
      {
!         case BOOLOID:
!             arg->func = PLyBool_FromString;
!             break;
!         case FLOAT4OID:
!         case FLOAT8OID:
!         case NUMERICOID:
!             arg->func = PLyFloat_FromString;
!             break;
!         case INT2OID:
!         case INT4OID:
!             arg->func = PLyInt_FromString;
!             break;
!         case INT8OID:
!             arg->func = PLyLong_FromString;
!             break;
          default:
+             arg->func = PLyString_FromString;
              break;
      }
  }

  void

Re: 7.3.3 - plpython & trigger problem

From
Arthur Ward
Date:
> > No, it does not match what I'm seeing. To check, I dropped my test
> > table and function, exited psql, started psql, and ran the script I
> > originally posted. I still get the same results. If I had to guess, this
> > seems more like a Postgres to plpython conversion problem than a
> > caching problem.
>
> Bingo.  The conversion-method-selection code is not only broken (for
> several datatypes besides this one), but quite wrongheaded (testing on
> the name of the datatype is the wrong approach).  I've applied the
> attached patch; please confirm it works for you.

Applied successfully to 7.3.3 and the bug in question is fixed for my
situation. Thanks a bunch for the quick fix!

Now to implement the workaround for the plpython trigger &
rowtype caching problem...

--
Arthur Ward