Hi,
With commit 4b93f57999a2ca9b9c9e573ea32ab1aeaa8bf496, which plpgsql
use its DTYPE_REC code paths for composite-type variables - below
test started failing with "invalid memory alloc request size 2139062167
<%28213%29%20906-2167>"
error.
Testcase:
create table foo ( name varchar(20), type varchar(20));
insert into foo values ( 'Ford', 'Car');
CREATE OR REPLACE FUNCTION Trigger_Function() returns trigger as
$$
BEGIN
RAISE NOTICE 'OLD: %, NEW: %', OLD, NEW;
IF NEW.name = 'Ford' THEN
return OLD; -- return old tuple
END IF;
return NEW; -- return original tuple
END;
$$ language plpgsql;
CREATE TRIGGER Before_Update_Trigger BEFORE UPDATE ON foo FOR EACH ROW
EXECUTE PROCEDURE Trigger_Function();
UPDATE foo SET type = 'New Car' where name = 'Ford';
Error coming while trying to copy the invalid tuple from
(heap_copytuple() <- ExecCopySlotTuple() <- ExecMaterializeSlot() <-
ExecUpdate() <- ExecModifyTable())
Here ExecBRUpdateTriggers() returns the invalid tuple when trigger
return old tuple. Looking further I found that tuple is getting free
at ExecBRUpdateTriggers(), below code:
if (trigtuple != fdw_trigtuple)
heap_freetuple(trigtuple);
It seems like before commit 4b93f57999a2ca9b9c9e573ea32ab1aeaa8bf496,
plpgsql_exec_trigger() always used to copy the old and new tuple but
after that commit it doen't copy the "old" and "new" tuple if
if user just did "return new" or "return old" without changing anything.
With commit 4b93f57999a2ca9b9c9e573ea32ab1aeaa8bf496, which plpgsql
use its DTYPE_REC code paths for composite-type variables - below
test started failing with "invalid memory alloc request size 2139062167"
error.
Testcase:
create table foo ( name varchar(20), type varchar(20));
insert into foo values ( 'Ford', 'Car');
CREATE OR REPLACE FUNCTION Trigger_Function() returns trigger as
$$
BEGIN
RAISE NOTICE 'OLD: %, NEW: %', OLD, NEW;
IF NEW.name = 'Ford' THEN
return OLD; -- return old tuple
END IF;
return NEW; -- return original tuple
END;
$$ language plpgsql;
CREATE TRIGGER Before_Update_Trigger BEFORE UPDATE ON foo FOR EACH ROW
EXECUTE PROCEDURE Trigger_Function();
UPDATE foo SET type = 'New Car' where name = 'Ford';
Error coming while trying to copy the invalid tuple from
(heap_copytuple() <- ExecCopySlotTuple() <- ExecMaterializeSlot() <-
ExecUpdate() <- ExecModifyTable())
Here ExecBRUpdateTriggers() returns the invalid tuple when trigger
return old tuple. Looking further I found that tuple is getting free
at ExecBRUpdateTriggers(), below code:
if (trigtuple != fdw_trigtuple)
heap_freetuple(trigtuple);
It seems like before commit 4b93f57999a2ca9b9c9e573ea32ab1aeaa8bf496,
plpgsql_exec_trigger() always used to copy the old and new tuple but
after that commit it doen't copy the "old" and "new" tuple if
if user just did "return new" or "return old" without changing anything.
+ /*
+ * Copy tuple to upper executor memory. But if user just did
+ * "return new" or "return old" without changing anything,
there's
+ * no need to copy; we can return the original tuple (which will
+ * save a few cycles in trigger.c as well as here).
+ */
+ if (rettup != trigdata->tg_newtuple &&
+ rettup != trigdata->tg_trigtuple)
+ rettup = SPI_copytuple(rettup);
In ExecBRUpdateTriggers(), we need to add a check that if trigtuple is same
as newtuple, then we don't require to free the trigtuple.
ExecBRDeleteTriggers() also does the similar things, but their we don't
need a check because it doesn't care about the return tuple.
PFA patch which add a check to not free the trigtuple if newtuple is same
as trigtuple and also added the related testcase.
Thanks,
Rushabh Lathia
www.EnterpriseDB.com
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index fffc009..9315244 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -2815,7 +2815,8 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
return NULL; /* "do nothing" */
}
}
- if (trigtuple != fdw_trigtuple)
+ if (trigtuple != fdw_trigtuple &&
+ trigtuple != newtuple)
heap_freetuple(trigtuple);
if (newtuple != slottuple)
diff --git a/src/pl/plpgsql/src/expected/plpgsql_record.out b/src/pl/plpgsql/src/expected/plpgsql_record.out
index 29e42fd..0e898c2 100644
--- a/src/pl/plpgsql/src/expected/plpgsql_record.out
+++ b/src/pl/plpgsql/src/expected/plpgsql_record.out
@@ -479,6 +479,25 @@ table mutable;
bar baz
(2 rows)
+-- check returning old tuple
+create or replace function sillytrig() returns trigger language plpgsql as
+$$begin
+ raise notice 'old.ctid = %', old.ctid;
+ raise notice 'old.tableoid = %', old.tableoid::regclass;
+ return old;
+end$$;
+update mutable set f2 = f2 || ' baz';
+NOTICE: old.ctid = (0,3)
+NOTICE: old.tableoid = mutable
+NOTICE: old.ctid = (0,4)
+NOTICE: old.tableoid = mutable
+table mutable;
+ f2
+---------
+ foo baz
+ bar baz
+(2 rows)
+
-- check returning a composite datum from a trigger
create or replace function sillytrig() returns trigger language plpgsql as
$$begin
diff --git a/src/pl/plpgsql/src/sql/plpgsql_record.sql b/src/pl/plpgsql/src/sql/plpgsql_record.sql
index 781ccb0..0b5eaed 100644
--- a/src/pl/plpgsql/src/sql/plpgsql_record.sql
+++ b/src/pl/plpgsql/src/sql/plpgsql_record.sql
@@ -310,6 +310,16 @@ insert into mutable values ('foo'), ('bar');
update mutable set f2 = f2 || ' baz';
table mutable;
+-- check returning old tuple
+create or replace function sillytrig() returns trigger language plpgsql as
+$$begin
+ raise notice 'old.ctid = %', old.ctid;
+ raise notice 'old.tableoid = %', old.tableoid::regclass;
+ return old;
+end$$;
+update mutable set f2 = f2 || ' baz';
+table mutable;
+
-- check returning a composite datum from a trigger
create or replace function sillytrig() returns trigger language plpgsql as
