pl/python invalidate functions with composite arguments
Here's a patch implementing properly invalidating functions that have
composite type arguments after the type changes, as mentioned in
http://archives.postgresql.org/pgsql-hackers/2010-12/msg01991.php. It's
an incremental patch on top of the plpython-refactor patch sent eariler.
Git branch for this patch:
https://github.com/wulczer/postgres/tree/invalidate-composites.
The idea in general is for this to work (taken straight from the unit
tests, btw)
CREATE TABLE employee (
name text,
basesalary integer,
bonus integer
);
INSERT INTO employee VALUES ('John', 100, 10), ('Mary', 200, 10);
CREATE OR REPLACE FUNCTION test_composite_table_input(e employee)
RETURNS integer AS $$
return e['basesalary'] + e['bonus']
$$ LANGUAGE plpythonu;
SELECT name, test_composite_table_input(employee.*) FROM employee;
ALTER TABLE employee DROP bonus;
-- this fails
SELECT name, test_composite_table_input(employee.*) FROM employee;
ALTER TABLE employee ADD bonus integer;
UPDATE employee SET bonus = 10;
-- this works again
SELECT name, test_composite_table_input(employee.*) FROM employee;
It's a long-standing TODO item, and a generally good thing to do.
Cheers,
Jan
Attachments:
plpython-invalidate-composites.difftext/x-patch; name=plpython-invalidate-composites.diffDownload
diff --git a/src/pl/plpython/expected/plpython_types.out b/src/pl/plpython/expected/plpython_types.out
index 982005b..cf7b7de 100644
*** a/src/pl/plpython/expected/plpython_types.out
--- b/src/pl/plpython/expected/plpython_types.out
*************** SELECT * FROM test_type_conversion_array
*** 599,604 ****
--- 599,656 ----
ERROR: return value of function with array return type is not a Python sequence
CONTEXT: while creating return value
PL/Python function "test_type_conversion_array_error"
+ ---
+ --- Composite types
+ ---
+ CREATE TABLE employee (
+ name text,
+ basesalary integer,
+ bonus integer
+ );
+ INSERT INTO employee VALUES ('John', 100, 10), ('Mary', 200, 10);
+ CREATE OR REPLACE FUNCTION test_composite_table_input(e employee) RETURNS integer AS $$
+ return e['basesalary'] + e['bonus']
+ $$ LANGUAGE plpythonu;
+ SELECT name, test_composite_table_input(employee.*) FROM employee;
+ name | test_composite_table_input
+ ------+----------------------------
+ John | 110
+ Mary | 210
+ (2 rows)
+
+ ALTER TABLE employee DROP bonus;
+ SELECT name, test_composite_table_input(employee.*) FROM employee;
+ ERROR: KeyError: 'bonus'
+ CONTEXT: PL/Python function "test_composite_table_input"
+ ALTER TABLE employee ADD bonus integer;
+ UPDATE employee SET bonus = 10;
+ SELECT name, test_composite_table_input(employee.*) FROM employee;
+ name | test_composite_table_input
+ ------+----------------------------
+ John | 110
+ Mary | 210
+ (2 rows)
+
+ CREATE TYPE named_pair AS (
+ i integer,
+ j integer
+ );
+ CREATE OR REPLACE FUNCTION test_composite_type_input(p named_pair) RETURNS integer AS $$
+ return sum(p.values())
+ $$ LANGUAGE plpythonu;
+ SELECT test_composite_type_input(row(1, 2));
+ test_composite_type_input
+ ---------------------------
+ 3
+ (1 row)
+
+ ALTER TYPE named_pair RENAME TO named_pair_2;
+ SELECT test_composite_type_input(row(1, 2));
+ test_composite_type_input
+ ---------------------------
+ 3
+ (1 row)
+
--
-- Prepared statements
--
diff --git a/src/pl/plpython/expected/plpython_types_3.out b/src/pl/plpython/expected/plpython_types_3.out
index eeb23b7..e10b060 100644
*** a/src/pl/plpython/expected/plpython_types_3.out
--- b/src/pl/plpython/expected/plpython_types_3.out
*************** SELECT * FROM test_type_conversion_array
*** 599,604 ****
--- 599,656 ----
ERROR: return value of function with array return type is not a Python sequence
CONTEXT: while creating return value
PL/Python function "test_type_conversion_array_error"
+ ---
+ --- Composite types
+ ---
+ CREATE TABLE employee (
+ name text,
+ basesalary integer,
+ bonus integer
+ );
+ INSERT INTO employee VALUES ('John', 100, 10), ('Mary', 200, 10);
+ CREATE OR REPLACE FUNCTION test_composite_table_input(e employee) RETURNS integer AS $$
+ return e['basesalary'] + e['bonus']
+ $$ LANGUAGE plpython3u;
+ SELECT name, test_composite_table_input(employee.*) FROM employee;
+ name | test_composite_table_input
+ ------+----------------------------
+ John | 110
+ Mary | 210
+ (2 rows)
+
+ ALTER TABLE employee DROP bonus;
+ SELECT name, test_composite_table_input(employee.*) FROM employee;
+ ERROR: KeyError: 'bonus'
+ CONTEXT: PL/Python function "test_composite_table_input"
+ ALTER TABLE employee ADD bonus integer;
+ UPDATE employee SET bonus = 10;
+ SELECT name, test_composite_table_input(employee.*) FROM employee;
+ name | test_composite_table_input
+ ------+----------------------------
+ John | 110
+ Mary | 210
+ (2 rows)
+
+ CREATE TYPE named_pair AS (
+ i integer,
+ j integer
+ );
+ CREATE OR REPLACE FUNCTION test_composite_type_input(p named_pair) RETURNS integer AS $$
+ return sum(p.values())
+ $$ LANGUAGE plpython3u;
+ SELECT test_composite_type_input(row(1, 2));
+ test_composite_type_input
+ ---------------------------
+ 3
+ (1 row)
+
+ ALTER TYPE named_pair RENAME TO named_pair_2;
+ SELECT test_composite_type_input(row(1, 2));
+ test_composite_type_input
+ ---------------------------
+ 3
+ (1 row)
+
--
-- Prepared statements
--
diff --git a/src/pl/plpython/plpython.c b/src/pl/plpython/plpython.c
index 67eb0f3..b1442d7 100644
*** a/src/pl/plpython/plpython.c
--- b/src/pl/plpython/plpython.c
*************** typedef int Py_ssize_t;
*** 101,106 ****
--- 101,107 ----
#include "nodes/makefuncs.h"
#include "parser/parse_type.h"
#include "tcop/tcopprot.h"
+ #include "access/transam.h"
#include "utils/builtins.h"
#include "utils/lsyscache.h"
#include "utils/memutils.h"
*************** typedef struct PLyTypeInfo
*** 192,201 ****
* is_rowtype can be: -1 = not known yet (initial state); 0 = scalar
* datatype; 1 = rowtype; 2 = rowtype, but I/O functions not set up yet
*/
! int is_rowtype;
} PLyTypeInfo;
-
/* cached procedure data */
typedef struct PLyProcedure
{
--- 193,205 ----
* is_rowtype can be: -1 = not known yet (initial state); 0 = scalar
* datatype; 1 = rowtype; 2 = rowtype, but I/O functions not set up yet
*/
! int is_rowtype;
! /* used to check if the type has been modified */
! Oid typ_relid;
! TransactionId typrel_xmin;
! ItemPointerData typrel_tid;
} PLyTypeInfo;
/* cached procedure data */
typedef struct PLyProcedure
{
*************** PLy_function_delete_args(PLyProcedure *p
*** 1286,1291 ****
--- 1290,1298 ----
static bool
PLy_procedure_valid(PLyProcedure *proc, HeapTuple procTup)
{
+ int i;
+ bool valid;
+
Assert(proc != NULL);
/* If the pg_proc tuple has changed, it's not valid */
*************** PLy_procedure_valid(PLyProcedure *proc,
*** 1293,1299 ****
ItemPointerEquals(&proc->fn_tid, &procTup->t_self)))
return false;
! return true;
}
/*
--- 1300,1339 ----
ItemPointerEquals(&proc->fn_tid, &procTup->t_self)))
return false;
! valid = true;
! /* If there are composite input arguments, they might have changed */
! for (i = 0; i < proc->nargs; i++)
! {
! Oid relid;
! HeapTuple relTup;
!
! /* Short-circuit on first changed argument */
! if (!valid)
! break;
!
! /* Only check input arguments that are composite */
! if (proc->args[i].is_rowtype != 1)
! continue;
!
! Assert(OidIsValid(proc->args[i].typ_relid));
! Assert(TransactionIdIsValid(proc->args[i].typrel_xmin));
! Assert(ItemPointerIsValid(&proc->args[i].typrel_tid));
!
! /* Get the pg_class tuple for the argument type */
! relid = proc->args[i].typ_relid;
! relTup = SearchSysCache1(RELOID, ObjectIdGetDatum(relid));
! if (!HeapTupleIsValid(relTup))
! elog(ERROR, "cache lookup failed for relation %u", relid);
!
! /* If it has changed, the function is not valid */
! if (!(proc->args[i].typrel_xmin == HeapTupleHeaderGetXmin(relTup->t_data) &&
! ItemPointerEquals(&proc->args[i].typrel_tid, &relTup->t_self)))
! valid = false;
!
! ReleaseSysCache(relTup);
! }
!
! return valid;
}
/*
*************** PLy_input_tuple_funcs(PLyTypeInfo *arg,
*** 1712,1717 ****
--- 1752,1783 ----
arg->in.r.atts = PLy_malloc0(desc->natts * sizeof(PLyDatumToOb));
}
+ /* Can this be an unnamed tuple? If not, then an Assert would be enough */
+ if (desc->tdtypmod != -1)
+ elog(ERROR, "received unnamed record type as input");
+
+ Assert(OidIsValid(desc->tdtypeid));
+ /*
+ * RECORDOID means we got called to create input functions for a tuple
+ * fetched by plpy.execute or for an anonymous record type
+ */
+ if (desc->tdtypeid != RECORDOID && !TransactionIdIsValid(arg->typrel_xmin))
+ {
+ HeapTuple relTup;
+
+ /* Get the pg_class tuple corresponding to the type of the input */
+ arg->typ_relid = typeidTypeRelid(desc->tdtypeid);
+ relTup = SearchSysCache1(RELOID, ObjectIdGetDatum(arg->typ_relid));
+ if (!HeapTupleIsValid(relTup))
+ elog(ERROR, "cache lookup failed for relation %u", arg->typ_relid);
+
+ /* Extract the XMIN value to later use it in PLy_procedure_valid */
+ arg->typrel_xmin = HeapTupleHeaderGetXmin(relTup->t_data);
+ arg->typrel_tid = relTup->t_self;
+
+ ReleaseSysCache(relTup);
+ }
+
for (i = 0; i < desc->natts; i++)
{
HeapTuple typeTup;
*************** PLy_typeinfo_init(PLyTypeInfo *arg)
*** 1916,1921 ****
--- 1982,1990 ----
arg->in.r.natts = arg->out.r.natts = 0;
arg->in.r.atts = NULL;
arg->out.r.atts = NULL;
+ arg->typ_relid = InvalidOid;
+ arg->typrel_xmin = InvalidTransactionId;
+ ItemPointerSetInvalid(&arg->typrel_tid);
}
static void
diff --git a/src/pl/plpython/sql/plpython_types.sql b/src/pl/plpython/sql/plpython_types.sql
index 2afc2ff..87d7553 100644
*** a/src/pl/plpython/sql/plpython_types.sql
--- b/src/pl/plpython/sql/plpython_types.sql
*************** $$ LANGUAGE plpythonu;
*** 278,283 ****
--- 278,323 ----
SELECT * FROM test_type_conversion_array_error();
+ ---
+ --- Composite types
+ ---
+ CREATE TABLE employee (
+ name text,
+ basesalary integer,
+ bonus integer
+ );
+
+ INSERT INTO employee VALUES ('John', 100, 10), ('Mary', 200, 10);
+
+ CREATE OR REPLACE FUNCTION test_composite_table_input(e employee) RETURNS integer AS $$
+ return e['basesalary'] + e['bonus']
+ $$ LANGUAGE plpythonu;
+
+ SELECT name, test_composite_table_input(employee.*) FROM employee;
+
+ ALTER TABLE employee DROP bonus;
+
+ SELECT name, test_composite_table_input(employee.*) FROM employee;
+
+ ALTER TABLE employee ADD bonus integer;
+ UPDATE employee SET bonus = 10;
+
+ SELECT name, test_composite_table_input(employee.*) FROM employee;
+
+ CREATE TYPE named_pair AS (
+ i integer,
+ j integer
+ );
+
+ CREATE OR REPLACE FUNCTION test_composite_type_input(p named_pair) RETURNS integer AS $$
+ return sum(p.values())
+ $$ LANGUAGE plpythonu;
+
+ SELECT test_composite_type_input(row(1, 2));
+
+ ALTER TYPE named_pair RENAME TO named_pair_2;
+
+ SELECT test_composite_type_input(row(1, 2));
--
-- Prepared statements
On 23/12/10 14:50, Jan Urbański wrote:
Here's a patch implementing properly invalidating functions that have
composite type arguments after the type changes, as mentioned in
http://archives.postgresql.org/pgsql-hackers/2010-12/msg01991.php. It's
an incremental patch on top of the plpython-refactor patch sent eariler.
Updated to master.
Attachments:
plpython-invalidate-composites.patchtext/x-patch; name=plpython-invalidate-composites.patchDownload
diff --git a/src/pl/plpython/expected/plpython_types.out b/src/pl/plpython/expected/plpython_types.out
index 982005b..cf7b7de 100644
*** a/src/pl/plpython/expected/plpython_types.out
--- b/src/pl/plpython/expected/plpython_types.out
*************** SELECT * FROM test_type_conversion_array
*** 599,604 ****
--- 599,656 ----
ERROR: return value of function with array return type is not a Python sequence
CONTEXT: while creating return value
PL/Python function "test_type_conversion_array_error"
+ ---
+ --- Composite types
+ ---
+ CREATE TABLE employee (
+ name text,
+ basesalary integer,
+ bonus integer
+ );
+ INSERT INTO employee VALUES ('John', 100, 10), ('Mary', 200, 10);
+ CREATE OR REPLACE FUNCTION test_composite_table_input(e employee) RETURNS integer AS $$
+ return e['basesalary'] + e['bonus']
+ $$ LANGUAGE plpythonu;
+ SELECT name, test_composite_table_input(employee.*) FROM employee;
+ name | test_composite_table_input
+ ------+----------------------------
+ John | 110
+ Mary | 210
+ (2 rows)
+
+ ALTER TABLE employee DROP bonus;
+ SELECT name, test_composite_table_input(employee.*) FROM employee;
+ ERROR: KeyError: 'bonus'
+ CONTEXT: PL/Python function "test_composite_table_input"
+ ALTER TABLE employee ADD bonus integer;
+ UPDATE employee SET bonus = 10;
+ SELECT name, test_composite_table_input(employee.*) FROM employee;
+ name | test_composite_table_input
+ ------+----------------------------
+ John | 110
+ Mary | 210
+ (2 rows)
+
+ CREATE TYPE named_pair AS (
+ i integer,
+ j integer
+ );
+ CREATE OR REPLACE FUNCTION test_composite_type_input(p named_pair) RETURNS integer AS $$
+ return sum(p.values())
+ $$ LANGUAGE plpythonu;
+ SELECT test_composite_type_input(row(1, 2));
+ test_composite_type_input
+ ---------------------------
+ 3
+ (1 row)
+
+ ALTER TYPE named_pair RENAME TO named_pair_2;
+ SELECT test_composite_type_input(row(1, 2));
+ test_composite_type_input
+ ---------------------------
+ 3
+ (1 row)
+
--
-- Prepared statements
--
diff --git a/src/pl/plpython/expected/plpython_types_3.out b/src/pl/plpython/expected/plpython_types_3.out
index eeb23b7..e10b060 100644
*** a/src/pl/plpython/expected/plpython_types_3.out
--- b/src/pl/plpython/expected/plpython_types_3.out
*************** SELECT * FROM test_type_conversion_array
*** 599,604 ****
--- 599,656 ----
ERROR: return value of function with array return type is not a Python sequence
CONTEXT: while creating return value
PL/Python function "test_type_conversion_array_error"
+ ---
+ --- Composite types
+ ---
+ CREATE TABLE employee (
+ name text,
+ basesalary integer,
+ bonus integer
+ );
+ INSERT INTO employee VALUES ('John', 100, 10), ('Mary', 200, 10);
+ CREATE OR REPLACE FUNCTION test_composite_table_input(e employee) RETURNS integer AS $$
+ return e['basesalary'] + e['bonus']
+ $$ LANGUAGE plpython3u;
+ SELECT name, test_composite_table_input(employee.*) FROM employee;
+ name | test_composite_table_input
+ ------+----------------------------
+ John | 110
+ Mary | 210
+ (2 rows)
+
+ ALTER TABLE employee DROP bonus;
+ SELECT name, test_composite_table_input(employee.*) FROM employee;
+ ERROR: KeyError: 'bonus'
+ CONTEXT: PL/Python function "test_composite_table_input"
+ ALTER TABLE employee ADD bonus integer;
+ UPDATE employee SET bonus = 10;
+ SELECT name, test_composite_table_input(employee.*) FROM employee;
+ name | test_composite_table_input
+ ------+----------------------------
+ John | 110
+ Mary | 210
+ (2 rows)
+
+ CREATE TYPE named_pair AS (
+ i integer,
+ j integer
+ );
+ CREATE OR REPLACE FUNCTION test_composite_type_input(p named_pair) RETURNS integer AS $$
+ return sum(p.values())
+ $$ LANGUAGE plpython3u;
+ SELECT test_composite_type_input(row(1, 2));
+ test_composite_type_input
+ ---------------------------
+ 3
+ (1 row)
+
+ ALTER TYPE named_pair RENAME TO named_pair_2;
+ SELECT test_composite_type_input(row(1, 2));
+ test_composite_type_input
+ ---------------------------
+ 3
+ (1 row)
+
--
-- Prepared statements
--
diff --git a/src/pl/plpython/plpython.c b/src/pl/plpython/plpython.c
index aafe556..54605fc 100644
*** a/src/pl/plpython/plpython.c
--- b/src/pl/plpython/plpython.c
*************** typedef int Py_ssize_t;
*** 101,106 ****
--- 101,107 ----
#include "nodes/makefuncs.h"
#include "parser/parse_type.h"
#include "tcop/tcopprot.h"
+ #include "access/transam.h"
#include "utils/builtins.h"
#include "utils/hsearch.h"
#include "utils/lsyscache.h"
*************** typedef struct PLyTypeInfo
*** 193,202 ****
* is_rowtype can be: -1 = not known yet (initial state); 0 = scalar
* datatype; 1 = rowtype; 2 = rowtype, but I/O functions not set up yet
*/
! int is_rowtype;
} PLyTypeInfo;
-
/* cached procedure data */
typedef struct PLyProcedure
{
--- 194,206 ----
* is_rowtype can be: -1 = not known yet (initial state); 0 = scalar
* datatype; 1 = rowtype; 2 = rowtype, but I/O functions not set up yet
*/
! int is_rowtype;
! /* used to check if the type has been modified */
! Oid typ_relid;
! TransactionId typrel_xmin;
! ItemPointerData typrel_tid;
} PLyTypeInfo;
/* cached procedure data */
typedef struct PLyProcedure
{
*************** PLy_function_delete_args(PLyProcedure *p
*** 1290,1300 ****
static bool
PLy_procedure_valid(PLyProcedure *proc, HeapTuple procTup)
{
Assert(proc != NULL);
/* If the pg_proc tuple has changed, it's not valid */
! return (proc->fn_xmin == HeapTupleHeaderGetXmin(procTup->t_data) &&
! ItemPointerEquals(&proc->fn_tid, &procTup->t_self));
}
--- 1294,1343 ----
static bool
PLy_procedure_valid(PLyProcedure *proc, HeapTuple procTup)
{
+ int i;
+ bool valid;
+
Assert(proc != NULL);
/* If the pg_proc tuple has changed, it's not valid */
! if (!(proc->fn_xmin == HeapTupleHeaderGetXmin(procTup->t_data) &&
! ItemPointerEquals(&proc->fn_tid, &procTup->t_self)))
! return false;
!
! valid = true;
! /* If there are composite input arguments, they might have changed */
! for (i = 0; i < proc->nargs; i++)
! {
! Oid relid;
! HeapTuple relTup;
!
! /* Short-circuit on first changed argument */
! if (!valid)
! break;
!
! /* Only check input arguments that are composite */
! if (proc->args[i].is_rowtype != 1)
! continue;
!
! Assert(OidIsValid(proc->args[i].typ_relid));
! Assert(TransactionIdIsValid(proc->args[i].typrel_xmin));
! Assert(ItemPointerIsValid(&proc->args[i].typrel_tid));
!
! /* Get the pg_class tuple for the argument type */
! relid = proc->args[i].typ_relid;
! relTup = SearchSysCache1(RELOID, ObjectIdGetDatum(relid));
! if (!HeapTupleIsValid(relTup))
! elog(ERROR, "cache lookup failed for relation %u", relid);
!
! /* If it has changed, the function is not valid */
! if (!(proc->args[i].typrel_xmin == HeapTupleHeaderGetXmin(relTup->t_data) &&
! ItemPointerEquals(&proc->args[i].typrel_tid, &relTup->t_self)))
! valid = false;
!
! ReleaseSysCache(relTup);
! }
!
! return valid;
}
*************** PLy_input_tuple_funcs(PLyTypeInfo *arg,
*** 1702,1707 ****
--- 1745,1776 ----
arg->in.r.atts = PLy_malloc0(desc->natts * sizeof(PLyDatumToOb));
}
+ /* Can this be an unnamed tuple? If not, then an Assert would be enough */
+ if (desc->tdtypmod != -1)
+ elog(ERROR, "received unnamed record type as input");
+
+ Assert(OidIsValid(desc->tdtypeid));
+ /*
+ * RECORDOID means we got called to create input functions for a tuple
+ * fetched by plpy.execute or for an anonymous record type
+ */
+ if (desc->tdtypeid != RECORDOID && !TransactionIdIsValid(arg->typrel_xmin))
+ {
+ HeapTuple relTup;
+
+ /* Get the pg_class tuple corresponding to the type of the input */
+ arg->typ_relid = typeidTypeRelid(desc->tdtypeid);
+ relTup = SearchSysCache1(RELOID, ObjectIdGetDatum(arg->typ_relid));
+ if (!HeapTupleIsValid(relTup))
+ elog(ERROR, "cache lookup failed for relation %u", arg->typ_relid);
+
+ /* Extract the XMIN value to later use it in PLy_procedure_valid */
+ arg->typrel_xmin = HeapTupleHeaderGetXmin(relTup->t_data);
+ arg->typrel_tid = relTup->t_self;
+
+ ReleaseSysCache(relTup);
+ }
+
for (i = 0; i < desc->natts; i++)
{
HeapTuple typeTup;
*************** PLy_typeinfo_init(PLyTypeInfo *arg)
*** 1906,1911 ****
--- 1975,1983 ----
arg->in.r.natts = arg->out.r.natts = 0;
arg->in.r.atts = NULL;
arg->out.r.atts = NULL;
+ arg->typ_relid = InvalidOid;
+ arg->typrel_xmin = InvalidTransactionId;
+ ItemPointerSetInvalid(&arg->typrel_tid);
}
static void
diff --git a/src/pl/plpython/sql/plpython_types.sql b/src/pl/plpython/sql/plpython_types.sql
index 2afc2ff..87d7553 100644
*** a/src/pl/plpython/sql/plpython_types.sql
--- b/src/pl/plpython/sql/plpython_types.sql
*************** $$ LANGUAGE plpythonu;
*** 278,283 ****
--- 278,323 ----
SELECT * FROM test_type_conversion_array_error();
+ ---
+ --- Composite types
+ ---
+ CREATE TABLE employee (
+ name text,
+ basesalary integer,
+ bonus integer
+ );
+
+ INSERT INTO employee VALUES ('John', 100, 10), ('Mary', 200, 10);
+
+ CREATE OR REPLACE FUNCTION test_composite_table_input(e employee) RETURNS integer AS $$
+ return e['basesalary'] + e['bonus']
+ $$ LANGUAGE plpythonu;
+
+ SELECT name, test_composite_table_input(employee.*) FROM employee;
+
+ ALTER TABLE employee DROP bonus;
+
+ SELECT name, test_composite_table_input(employee.*) FROM employee;
+
+ ALTER TABLE employee ADD bonus integer;
+ UPDATE employee SET bonus = 10;
+
+ SELECT name, test_composite_table_input(employee.*) FROM employee;
+
+ CREATE TYPE named_pair AS (
+ i integer,
+ j integer
+ );
+
+ CREATE OR REPLACE FUNCTION test_composite_type_input(p named_pair) RETURNS integer AS $$
+ return sum(p.values())
+ $$ LANGUAGE plpythonu;
+
+ SELECT test_composite_type_input(row(1, 2));
+
+ ALTER TYPE named_pair RENAME TO named_pair_2;
+
+ SELECT test_composite_type_input(row(1, 2));
--
-- Prepared statements
On 27/01/11 22:42, Jan Urbański wrote:
On 23/12/10 14:50, Jan Urbański wrote:
Here's a patch implementing properly invalidating functions that have
composite type arguments after the type changes, as mentioned in
http://archives.postgresql.org/pgsql-hackers/2010-12/msg01991.php. It's
an incremental patch on top of the plpython-refactor patch sent eariler.Updated to master.
Again.
Attachments:
plpython-invalidate-composites.patchtext/x-patch; name=plpython-invalidate-composites.patchDownload
diff --git a/src/pl/plpython/expected/plpython_types.out b/src/pl/plpython/expected/plpython_types.out
index e74a400..d5f2c70 100644
*** a/src/pl/plpython/expected/plpython_types.out
--- b/src/pl/plpython/expected/plpython_types.out
*************** SELECT * FROM test_type_conversion_array
*** 603,608 ****
--- 603,660 ----
ERROR: return value of function with array return type is not a Python sequence
CONTEXT: while creating return value
PL/Python function "test_type_conversion_array_error"
+ ---
+ --- Composite types
+ ---
+ CREATE TABLE employee (
+ name text,
+ basesalary integer,
+ bonus integer
+ );
+ INSERT INTO employee VALUES ('John', 100, 10), ('Mary', 200, 10);
+ CREATE OR REPLACE FUNCTION test_composite_table_input(e employee) RETURNS integer AS $$
+ return e['basesalary'] + e['bonus']
+ $$ LANGUAGE plpythonu;
+ SELECT name, test_composite_table_input(employee.*) FROM employee;
+ name | test_composite_table_input
+ ------+----------------------------
+ John | 110
+ Mary | 210
+ (2 rows)
+
+ ALTER TABLE employee DROP bonus;
+ SELECT name, test_composite_table_input(employee.*) FROM employee;
+ ERROR: KeyError: 'bonus'
+ CONTEXT: PL/Python function "test_composite_table_input"
+ ALTER TABLE employee ADD bonus integer;
+ UPDATE employee SET bonus = 10;
+ SELECT name, test_composite_table_input(employee.*) FROM employee;
+ name | test_composite_table_input
+ ------+----------------------------
+ John | 110
+ Mary | 210
+ (2 rows)
+
+ CREATE TYPE named_pair AS (
+ i integer,
+ j integer
+ );
+ CREATE OR REPLACE FUNCTION test_composite_type_input(p named_pair) RETURNS integer AS $$
+ return sum(p.values())
+ $$ LANGUAGE plpythonu;
+ SELECT test_composite_type_input(row(1, 2));
+ test_composite_type_input
+ ---------------------------
+ 3
+ (1 row)
+
+ ALTER TYPE named_pair RENAME TO named_pair_2;
+ SELECT test_composite_type_input(row(1, 2));
+ test_composite_type_input
+ ---------------------------
+ 3
+ (1 row)
+
--
-- Prepared statements
--
diff --git a/src/pl/plpython/expected/plpython_types_3.out b/src/pl/plpython/expected/plpython_types_3.out
index 577c1ff..ca81b08 100644
*** a/src/pl/plpython/expected/plpython_types_3.out
--- b/src/pl/plpython/expected/plpython_types_3.out
*************** SELECT * FROM test_type_conversion_array
*** 603,608 ****
--- 603,660 ----
ERROR: return value of function with array return type is not a Python sequence
CONTEXT: while creating return value
PL/Python function "test_type_conversion_array_error"
+ ---
+ --- Composite types
+ ---
+ CREATE TABLE employee (
+ name text,
+ basesalary integer,
+ bonus integer
+ );
+ INSERT INTO employee VALUES ('John', 100, 10), ('Mary', 200, 10);
+ CREATE OR REPLACE FUNCTION test_composite_table_input(e employee) RETURNS integer AS $$
+ return e['basesalary'] + e['bonus']
+ $$ LANGUAGE plpython3u;
+ SELECT name, test_composite_table_input(employee.*) FROM employee;
+ name | test_composite_table_input
+ ------+----------------------------
+ John | 110
+ Mary | 210
+ (2 rows)
+
+ ALTER TABLE employee DROP bonus;
+ SELECT name, test_composite_table_input(employee.*) FROM employee;
+ ERROR: KeyError: 'bonus'
+ CONTEXT: PL/Python function "test_composite_table_input"
+ ALTER TABLE employee ADD bonus integer;
+ UPDATE employee SET bonus = 10;
+ SELECT name, test_composite_table_input(employee.*) FROM employee;
+ name | test_composite_table_input
+ ------+----------------------------
+ John | 110
+ Mary | 210
+ (2 rows)
+
+ CREATE TYPE named_pair AS (
+ i integer,
+ j integer
+ );
+ CREATE OR REPLACE FUNCTION test_composite_type_input(p named_pair) RETURNS integer AS $$
+ return sum(p.values())
+ $$ LANGUAGE plpython3u;
+ SELECT test_composite_type_input(row(1, 2));
+ test_composite_type_input
+ ---------------------------
+ 3
+ (1 row)
+
+ ALTER TYPE named_pair RENAME TO named_pair_2;
+ SELECT test_composite_type_input(row(1, 2));
+ test_composite_type_input
+ ---------------------------
+ 3
+ (1 row)
+
--
-- Prepared statements
--
diff --git a/src/pl/plpython/plpython.c b/src/pl/plpython/plpython.c
index fff7de7..3c0f35b 100644
*** a/src/pl/plpython/plpython.c
--- b/src/pl/plpython/plpython.c
*************** typedef int Py_ssize_t;
*** 101,106 ****
--- 101,107 ----
#include "nodes/makefuncs.h"
#include "parser/parse_type.h"
#include "tcop/tcopprot.h"
+ #include "access/transam.h"
#include "access/xact.h"
#include "utils/builtins.h"
#include "utils/hsearch.h"
*************** typedef struct PLyTypeInfo
*** 194,203 ****
* is_rowtype can be: -1 = not known yet (initial state); 0 = scalar
* datatype; 1 = rowtype; 2 = rowtype, but I/O functions not set up yet
*/
! int is_rowtype;
} PLyTypeInfo;
-
/* cached procedure data */
typedef struct PLyProcedure
{
--- 195,207 ----
* is_rowtype can be: -1 = not known yet (initial state); 0 = scalar
* datatype; 1 = rowtype; 2 = rowtype, but I/O functions not set up yet
*/
! int is_rowtype;
! /* used to check if the type has been modified */
! Oid typ_relid;
! TransactionId typrel_xmin;
! ItemPointerData typrel_tid;
} PLyTypeInfo;
/* cached procedure data */
typedef struct PLyProcedure
{
*************** PLy_function_delete_args(PLyProcedure *p
*** 1330,1340 ****
static bool
PLy_procedure_valid(PLyProcedure *proc, HeapTuple procTup)
{
Assert(proc != NULL);
/* If the pg_proc tuple has changed, it's not valid */
! return (proc->fn_xmin == HeapTupleHeaderGetXmin(procTup->t_data) &&
! ItemPointerEquals(&proc->fn_tid, &procTup->t_self));
}
--- 1334,1383 ----
static bool
PLy_procedure_valid(PLyProcedure *proc, HeapTuple procTup)
{
+ int i;
+ bool valid;
+
Assert(proc != NULL);
/* If the pg_proc tuple has changed, it's not valid */
! if (!(proc->fn_xmin == HeapTupleHeaderGetXmin(procTup->t_data) &&
! ItemPointerEquals(&proc->fn_tid, &procTup->t_self)))
! return false;
!
! valid = true;
! /* If there are composite input arguments, they might have changed */
! for (i = 0; i < proc->nargs; i++)
! {
! Oid relid;
! HeapTuple relTup;
!
! /* Short-circuit on first changed argument */
! if (!valid)
! break;
!
! /* Only check input arguments that are composite */
! if (proc->args[i].is_rowtype != 1)
! continue;
!
! Assert(OidIsValid(proc->args[i].typ_relid));
! Assert(TransactionIdIsValid(proc->args[i].typrel_xmin));
! Assert(ItemPointerIsValid(&proc->args[i].typrel_tid));
!
! /* Get the pg_class tuple for the argument type */
! relid = proc->args[i].typ_relid;
! relTup = SearchSysCache1(RELOID, ObjectIdGetDatum(relid));
! if (!HeapTupleIsValid(relTup))
! elog(ERROR, "cache lookup failed for relation %u", relid);
!
! /* If it has changed, the function is not valid */
! if (!(proc->args[i].typrel_xmin == HeapTupleHeaderGetXmin(relTup->t_data) &&
! ItemPointerEquals(&proc->args[i].typrel_tid, &relTup->t_self)))
! valid = false;
!
! ReleaseSysCache(relTup);
! }
!
! return valid;
}
*************** PLy_input_tuple_funcs(PLyTypeInfo *arg,
*** 1742,1747 ****
--- 1785,1816 ----
arg->in.r.atts = PLy_malloc0(desc->natts * sizeof(PLyDatumToOb));
}
+ /* Can this be an unnamed tuple? If not, then an Assert would be enough */
+ if (desc->tdtypmod != -1)
+ elog(ERROR, "received unnamed record type as input");
+
+ Assert(OidIsValid(desc->tdtypeid));
+ /*
+ * RECORDOID means we got called to create input functions for a tuple
+ * fetched by plpy.execute or for an anonymous record type
+ */
+ if (desc->tdtypeid != RECORDOID && !TransactionIdIsValid(arg->typrel_xmin))
+ {
+ HeapTuple relTup;
+
+ /* Get the pg_class tuple corresponding to the type of the input */
+ arg->typ_relid = typeidTypeRelid(desc->tdtypeid);
+ relTup = SearchSysCache1(RELOID, ObjectIdGetDatum(arg->typ_relid));
+ if (!HeapTupleIsValid(relTup))
+ elog(ERROR, "cache lookup failed for relation %u", arg->typ_relid);
+
+ /* Extract the XMIN value to later use it in PLy_procedure_valid */
+ arg->typrel_xmin = HeapTupleHeaderGetXmin(relTup->t_data);
+ arg->typrel_tid = relTup->t_self;
+
+ ReleaseSysCache(relTup);
+ }
+
for (i = 0; i < desc->natts; i++)
{
HeapTuple typeTup;
*************** PLy_typeinfo_init(PLyTypeInfo *arg)
*** 1946,1951 ****
--- 2015,2023 ----
arg->in.r.natts = arg->out.r.natts = 0;
arg->in.r.atts = NULL;
arg->out.r.atts = NULL;
+ arg->typ_relid = InvalidOid;
+ arg->typrel_xmin = InvalidTransactionId;
+ ItemPointerSetInvalid(&arg->typrel_tid);
}
static void
diff --git a/src/pl/plpython/sql/plpython_types.sql b/src/pl/plpython/sql/plpython_types.sql
index 2afc2ff..87d7553 100644
*** a/src/pl/plpython/sql/plpython_types.sql
--- b/src/pl/plpython/sql/plpython_types.sql
*************** $$ LANGUAGE plpythonu;
*** 278,283 ****
--- 278,323 ----
SELECT * FROM test_type_conversion_array_error();
+ ---
+ --- Composite types
+ ---
+ CREATE TABLE employee (
+ name text,
+ basesalary integer,
+ bonus integer
+ );
+
+ INSERT INTO employee VALUES ('John', 100, 10), ('Mary', 200, 10);
+
+ CREATE OR REPLACE FUNCTION test_composite_table_input(e employee) RETURNS integer AS $$
+ return e['basesalary'] + e['bonus']
+ $$ LANGUAGE plpythonu;
+
+ SELECT name, test_composite_table_input(employee.*) FROM employee;
+
+ ALTER TABLE employee DROP bonus;
+
+ SELECT name, test_composite_table_input(employee.*) FROM employee;
+
+ ALTER TABLE employee ADD bonus integer;
+ UPDATE employee SET bonus = 10;
+
+ SELECT name, test_composite_table_input(employee.*) FROM employee;
+
+ CREATE TYPE named_pair AS (
+ i integer,
+ j integer
+ );
+
+ CREATE OR REPLACE FUNCTION test_composite_type_input(p named_pair) RETURNS integer AS $$
+ return sum(p.values())
+ $$ LANGUAGE plpythonu;
+
+ SELECT test_composite_type_input(row(1, 2));
+
+ ALTER TYPE named_pair RENAME TO named_pair_2;
+
+ SELECT test_composite_type_input(row(1, 2));
--
-- Prepared statements
On Wed, Feb 9, 2011 at 02:09, Jan Urbański <wulczer@wulczer.org> wrote:
On 27/01/11 22:42, Jan Urbański wrote:
On 23/12/10 14:50, Jan Urbański wrote:
Here's a patch implementing properly invalidating functions that have
composite type arguments after the type changes, as mentioned in
http://archives.postgresql.org/pgsql-hackers/2010-12/msg01991.php. It's
an incremental patch on top of the plpython-refactor patch sent eariler.Updated to master.
Again.
Looks good, it works as described, the code is clean and well
documented and it passes the added regression tests.
I took the liberty of looking at the other pls to see how they handled
this to find they don't cache them in the first place. For fun, I
hacked plpython to not cache to see if there was any performance
difference pre patch, post patch and in the non-cached cases. I
couldn't find any probably due to:
1) my simple test case (select
count(test_composite_table_input('(John, 100, "(10)")')) FROM
generate_series(1, 1000000);)
2) things being cached
3) cache invalidation having to do most of the work that the non
caching cache does. I think there is one or two more SearchSysCall's.
4) overhead from cassert
It seems a bit heavy handed to invalidate and remake the entire
plpython function whenever we hit this case. I think we could get away
with setting ->is_rowtype = 2 in PLy_procedure_valid() instead. I
suppose it should be fairly rare case anyway so... *shrug*.
On Thu, Feb 10, 2011 at 9:06 PM, Alex Hunsaker <badalex@gmail.com> wrote:
On Wed, Feb 9, 2011 at 02:09, Jan Urbański <wulczer@wulczer.org> wrote:
On 27/01/11 22:42, Jan Urbański wrote:
On 23/12/10 14:50, Jan Urbański wrote:
Here's a patch implementing properly invalidating functions that have
composite type arguments after the type changes, as mentioned in
http://archives.postgresql.org/pgsql-hackers/2010-12/msg01991.php. It's
an incremental patch on top of the plpython-refactor patch sent eariler.Updated to master.
Again.
Looks good, it works as described, the code is clean and well
documented and it passes the added regression tests.I took the liberty of looking at the other pls to see how they handled
this to find they don't cache them in the first place. For fun, I
hacked plpython to not cache to see if there was any performance
difference pre patch, post patch and in the non-cached cases. I
couldn't find any probably due to:
1) my simple test case (select
count(test_composite_table_input('(John, 100, "(10)")')) FROM
generate_series(1, 1000000);)
2) things being cached
3) cache invalidation having to do most of the work that the non
caching cache does. I think there is one or two more SearchSysCall's.
4) overhead from cassertIt seems a bit heavy handed to invalidate and remake the entire
plpython function whenever we hit this case. I think we could get away
with setting ->is_rowtype = 2 in PLy_procedure_valid() instead. I
suppose it should be fairly rare case anyway so... *shrug*.
This last comment might make me think that we're looking for a new
version of this patch, but the comment on the CommitFest application
says "looks good". So I'm not sure whether this should be marked
Waiting on Author or Ready for Committer, but the current status of
Needs Review looks wrong.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On 11/02/11 16:47, Robert Haas wrote:
On Thu, Feb 10, 2011 at 9:06 PM, Alex Hunsaker <badalex@gmail.com> wrote:
On Wed, Feb 9, 2011 at 02:09, Jan Urbański <wulczer@wulczer.org> wrote:
It seems a bit heavy handed to invalidate and remake the entire
plpython function whenever we hit this case. I think we could get away
with setting ->is_rowtype = 2 in PLy_procedure_valid() instead. I
suppose it should be fairly rare case anyway so... *shrug*.This last comment might make me think that we're looking for a new
version of this patch, but the comment on the CommitFest application
says "looks good". So I'm not sure whether this should be marked
Waiting on Author or Ready for Committer, but the current status of
Needs Review looks wrong.
I'm not planning on writing a new version of the patch. AIUI Alex said,
that there's a possible optimization to be done, but the gain is so
small that it doesn't matter.
Jan
On Fri, Feb 11, 2011 at 10:51 AM, Jan Urbański <wulczer@wulczer.org> wrote:
On 11/02/11 16:47, Robert Haas wrote:
On Thu, Feb 10, 2011 at 9:06 PM, Alex Hunsaker <badalex@gmail.com> wrote:
On Wed, Feb 9, 2011 at 02:09, Jan Urbański <wulczer@wulczer.org> wrote:
It seems a bit heavy handed to invalidate and remake the entire
plpython function whenever we hit this case. I think we could get away
with setting ->is_rowtype = 2 in PLy_procedure_valid() instead. I
suppose it should be fairly rare case anyway so... *shrug*.This last comment might make me think that we're looking for a new
version of this patch, but the comment on the CommitFest application
says "looks good". So I'm not sure whether this should be marked
Waiting on Author or Ready for Committer, but the current status of
Needs Review looks wrong.I'm not planning on writing a new version of the patch. AIUI Alex said,
that there's a possible optimization to be done, but the gain is so
small that it doesn't matter.
OK, marked Ready for Committer.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Fri, Feb 11, 2011 at 08:47, Robert Haas <robertmhaas@gmail.com> wrote:
On Thu, Feb 10, 2011 at 9:06 PM, Alex Hunsaker <badalex@gmail.com> wrote:
On Wed, Feb 9, 2011 at 02:09, Jan Urbański <wulczer@wulczer.org> wrote:
On 27/01/11 22:42, Jan Urbański wrote:
On 23/12/10 14:50, Jan Urbański wrote:
Here's a patch implementing properly invalidating functions that have
composite type arguments after the type changes, as mentioned in
http://archives.postgresql.org/pgsql-hackers/2010-12/msg01991.php. It's
an incremental patch on top of the plpython-refactor patch sent eariler.Updated to master.
Again.
Looks good, it works as described, the code is clean and well
documented and it passes the added regression tests.I took the liberty of looking at the other pls to see how they handled
this to find they don't cache them in the first place. For fun, I
hacked plpython to not cache to see if there was any performance
difference pre patch, post patch and in the non-cached cases. I
couldn't find any probably due to:
1) my simple test case (select
count(test_composite_table_input('(John, 100, "(10)")')) FROM
generate_series(1, 1000000);)
2) things being cached
3) cache invalidation having to do most of the work that the non
caching cache does. I think there is one or two more SearchSysCall's.
4) overhead from cassertIt seems a bit heavy handed to invalidate and remake the entire
plpython function whenever we hit this case. I think we could get away
with setting ->is_rowtype = 2 in PLy_procedure_valid() instead. I
suppose it should be fairly rare case anyway so... *shrug*.This last comment might make me think that we're looking for a new
version of this patch, but the comment on the CommitFest application
says "looks good". So I'm not sure whether this should be marked
Waiting on Author or Ready for Committer, but the current status of
Needs Review looks wrong.
Drat, Ive been found it. I just didn't want to make things to easy for you. :)
On ons, 2011-02-09 at 10:09 +0100, Jan Urbański wrote:
On 27/01/11 22:42, Jan Urbański wrote:
On 23/12/10 14:50, Jan Urbański wrote:
Here's a patch implementing properly invalidating functions that have
composite type arguments after the type changes, as mentioned in
http://archives.postgresql.org/pgsql-hackers/2010-12/msg01991.php. It's
an incremental patch on top of the plpython-refactor patch sent eariler.Updated to master.
Again.
Committed.