Fix update crash after code cleanup

Issue: the code cleanup introduced in PR percona#52 modified the original
tuple in the update method instead of decrypting the tuple data
into a copy. This caused data curruption crashes in some test.

Fix: reintroduce the missing palloc to the update method.

Fixes percona#62
Fixes percona#64

Makefile

@@ -6,7 +6,7 @@ EXTENSION = pg_tde
 DATA = pg_tde--1.0.sql
 
 REGRESS_OPTS = --temp-config $(top_srcdir)/contrib/postgres-tde-ext/postgres-tde-ext.conf
-REGRESS = non_sorted_off_compact update_compare_indexes pgtde_is_encrypted
+REGRESS = non_sorted_off_compact update_compare_indexes pgtde_is_encrypted trigger_on_view
 TAP_TESTS = 1
 
 OBJS = src/encryption/enc_tuple.o \

expected/trigger_on_view.out

@@ -0,0 +1,204 @@
+CREATE extension pg_tde;
+--
+-- 2 -- Test triggers on a join view
+--
+SET default_table_access_method TO 'pg_tde';
+DROP VIEW IF EXISTS city_view CASCADE;
+NOTICE:  view "city_view" does not exist, skipping
+DROP TABLE IF exists country_table CASCADE;
+NOTICE:  table "country_table" does not exist, skipping
+DROP TABLE IF exists city_table cascade;
+NOTICE:  table "city_table" does not exist, skipping
+ CREATE TABLE country_table (
+     country_id        serial primary key,
+     country_name    text unique not null,
+     continent        text not null
+ ) using pg_tde;
+
+ INSERT INTO country_table (country_name, continent)
+     VALUES ('Japan', 'Asia'),
+            ('UK', 'Europe'),
+            ('USA', 'North America')
+     RETURNING *;
+ country_id | country_name |   continent   
+------------+--------------+---------------
+          1 | Japan        | Asia
+          2 | UK           | Europe
+          3 | USA          | North America
+(3 rows)
+
+
+ CREATE TABLE city_table (
+     city_id        serial primary key,
+     city_name    text not null,
+     population    bigint, 
+     country_id    int references country_table
+ ) using pg_tde;
+
+ CREATE VIEW city_view AS
+     SELECT city_id, city_name, population, country_name, continent
+     FROM city_table ci
+     LEFT JOIN country_table co ON co.country_id = ci.country_id;
+
+CREATE OR REPLACE FUNCTION city_insert() RETURNS trigger LANGUAGE plpgsql AS $$
+ declare
+     ctry_id int;
+ begin
+     if NEW.country_name IS NOT NULL then
+         SELECT country_id, continent INTO ctry_id, NEW.continent
+             FROM country_table WHERE country_name = NEW.country_name;
+         if NOT FOUND then 
+             raise exception 'No such country: "%"', NEW.country_name;
+         end if;
+     else
+         NEW.continent := NULL;
+     end if;
+
+     if NEW.city_id IS NOT NULL then
+         INSERT INTO city_table
+             VALUES(NEW.city_id, NEW.city_name, NEW.population, ctry_id);
+     else
+         INSERT INTO city_table(city_name, population, country_id)
+             VALUES(NEW.city_name, NEW.population, ctry_id)
+             RETURNING city_id INTO NEW.city_id;
+     end if;
+
+     RETURN NEW;
+ end;
+ $$;
+  CREATE TRIGGER city_insert_trig INSTEAD OF INSERT ON city_view
+ FOR EACH ROW EXECUTE PROCEDURE city_insert();
+
+ CREATE OR REPLACE FUNCTION city_delete() RETURNS trigger LANGUAGE plpgsql AS $$
+ begin
+     DELETE FROM city_table WHERE city_id = OLD.city_id;
+     if NOT FOUND then RETURN NULL; end if;
+     RETURN OLD;
+ end;
+ $$;
+
+ CREATE TRIGGER city_delete_trig INSTEAD OF DELETE ON city_view
+ FOR EACH ROW EXECUTE PROCEDURE city_delete();
+
+ CREATE OR REPLACE FUNCTION city_update() RETURNS trigger LANGUAGE plpgsql AS $$
+ declare
+     ctry_id int;
+ begin
+     if NEW.country_name IS DISTINCT FROM OLD.country_name then
+         SELECT country_id, continent INTO ctry_id, NEW.continent
+             FROM country_table WHERE country_name = NEW.country_name;
+         if NOT FOUND then
+             raise exception 'No such country: "%"', NEW.country_name;
+         end if;
+
+         UPDATE city_table SET city_name = NEW.city_name,
+                               population = NEW.population,
+                               country_id = ctry_id
+             WHERE city_id = OLD.city_id;
+     else
+         UPDATE city_table SET city_name = NEW.city_name,
+                               population = NEW.population
+             WHERE city_id = OLD.city_id;
+         NEW.continent := OLD.continent;
+     end if;
+
+     if NOT FOUND then RETURN NULL; end if;
+     RETURN NEW;
+ end;
+ $$;
+ CREATE TRIGGER city_update_trig INSTEAD OF UPDATE ON city_view
+ FOR EACH ROW EXECUTE PROCEDURE city_update();
+
+-- INSERT .. RETURNING
+ INSERT INTO city_view(city_name) VALUES('Tokyo') RETURNING *;
+ city_id | city_name | population | country_name | continent 
+---------+-----------+------------+--------------+-----------
+       1 | Tokyo     |            |              | 
+(1 row)
+
+ INSERT INTO city_view(city_name, population) VALUES('London', 7556900) RETURNING *;
+ city_id | city_name | population | country_name | continent 
+---------+-----------+------------+--------------+-----------
+       2 | London    |    7556900 |              | 
+(1 row)
+
+ INSERT INTO city_view(city_name, country_name) VALUES('Washington DC', 'USA') RETURNING *;
+ city_id |   city_name   | population | country_name |   continent   
+---------+---------------+------------+--------------+---------------
+       3 | Washington DC |            | USA          | North America
+(1 row)
+
+ INSERT INTO city_view(city_id, city_name) VALUES(123456, 'New York') RETURNING *;
+ city_id | city_name | population | country_name | continent 
+---------+-----------+------------+--------------+-----------
+  123456 | New York  |            |              | 
+(1 row)
+
+ INSERT INTO city_view VALUES(234567, 'Birmingham', 1016800, 'UK', 'EU') RETURNING *;
+ city_id | city_name  | population | country_name | continent 
+---------+------------+------------+--------------+-----------
+  234567 | Birmingham |    1016800 | UK           | Europe
+(1 row)
+
+
+ -- UPDATE .. RETURNING
+ UPDATE city_view SET country_name = 'Japon' WHERE city_name = 'Tokyo'; -- error
+ERROR:  No such country: "Japon"
+CONTEXT:  PL/pgSQL function city_update() line 9 at RAISE
+ UPDATE city_view SET country_name = 'Japan' WHERE city_name = 'Takyo'; -- no match
+ UPDATE city_view SET country_name = 'Japan' WHERE city_name = 'Tokyo' RETURNING *; -- OK
+ city_id | city_name | population | country_name | continent 
+---------+-----------+------------+--------------+-----------
+       1 | Tokyo     |            | Japan        | Asia
+(1 row)
+
+
+ UPDATE city_view SET population = 13010279 WHERE city_name = 'Tokyo' RETURNING *;
+ city_id | city_name | population | country_name | continent 
+---------+-----------+------------+--------------+-----------
+       1 | Tokyo     |   13010279 | Japan        | Asia
+       1 | Tokyo     |   13010279 |              | 
+(2 rows)
+
+ UPDATE city_view SET country_name = 'UK' WHERE city_name = 'New York' RETURNING *;
+ city_id | city_name | population | country_name | continent 
+---------+-----------+------------+--------------+-----------
+  123456 | New York  |            | UK           | Europe
+(1 row)
+
+ UPDATE city_view SET country_name = 'USA', population = 8391881 WHERE city_name = 'New York' RETURNING *;
+ city_id | city_name | population | country_name |   continent   
+---------+-----------+------------+--------------+---------------
+  123456 | New York  |    8391881 | USA          | North America
+  123456 | New York  |    8391881 | USA          | North America
+(2 rows)
+
+ UPDATE city_view SET continent = 'EU' WHERE continent = 'Europe' RETURNING *;
+ city_id | city_name  | population | country_name | continent 
+---------+------------+------------+--------------+-----------
+  234567 | Birmingham |    1016800 | UK           | Europe
+  123456 | New York   |            | UK           | Europe
+(2 rows)
+
+ UPDATE city_view v1 SET country_name = v2.country_name FROM city_view v2
+     WHERE v2.city_name = 'Birmingham' AND v1.city_name = 'London' RETURNING *;
+ city_id | city_name | population | country_name | continent | city_id | city_name  | population | country_name | continent 
+---------+-----------+------------+--------------+-----------+---------+------------+------------+--------------+-----------
+       2 | London    |    7556900 | UK           | Europe    |  234567 | Birmingham |    1016800 | UK           | Europe
+       2 | London    |    7556900 | UK           | Europe    |  234567 | Birmingham |    1016800 | UK           | Europe
+(2 rows)
+
+
+ -- DELETE .. RETURNING
+ DELETE FROM city_view WHERE city_name = 'Birmingham' RETURNING *;
+ city_id | city_name  | population | country_name | continent 
+---------+------------+------------+--------------+-----------
+  234567 | Birmingham |    1016800 | UK           | Europe
+(1 row)
+
+
+DROP extension pg_tde CASCADE;
+NOTICE:  drop cascades to 3 other objects
+DETAIL:  drop cascades to table country_table
+drop cascades to table city_table
+drop cascades to view city_view

meson.build

@@ -71,6 +71,7 @@ tests += {
       'update_compare_indexes',
       'pgtde_is_encrypted',
       'multi_insert',
+      'trigger_on_view',
     ],
     'regress_args': ['--temp-config', files('postgres-tde-ext.conf')],
     'runningcheck': false,

sql/trigger_on_view.sql

@@ -0,0 +1,130 @@
+CREATE extension pg_tde;
+
+--
+-- 2 -- Test triggers on a join view
+--
+SET default_table_access_method TO 'pg_tde';
+
+DROP VIEW IF EXISTS city_view CASCADE;
+DROP TABLE IF exists country_table CASCADE;
+DROP TABLE IF exists city_table cascade;
+
+ CREATE TABLE country_table (
+     country_id        serial primary key,
+     country_name    text unique not null,
+     continent        text not null
+ ) using pg_tde;
+
+ INSERT INTO country_table (country_name, continent)
+     VALUES ('Japan', 'Asia'),
+            ('UK', 'Europe'),
+            ('USA', 'North America')
+     RETURNING *;
+
+ CREATE TABLE city_table (
+     city_id        serial primary key,
+     city_name    text not null,
+     population    bigint, 
+     country_id    int references country_table
+ ) using pg_tde;
+
+ CREATE VIEW city_view AS
+     SELECT city_id, city_name, population, country_name, continent
+     FROM city_table ci
+     LEFT JOIN country_table co ON co.country_id = ci.country_id;
+
+CREATE OR REPLACE FUNCTION city_insert() RETURNS trigger LANGUAGE plpgsql AS $$
+ declare
+     ctry_id int;
+ begin
+     if NEW.country_name IS NOT NULL then
+         SELECT country_id, continent INTO ctry_id, NEW.continent
+             FROM country_table WHERE country_name = NEW.country_name;
+         if NOT FOUND then 
+             raise exception 'No such country: "%"', NEW.country_name;
+         end if;
+     else
+         NEW.continent := NULL;
+     end if;
+
+     if NEW.city_id IS NOT NULL then
+         INSERT INTO city_table
+             VALUES(NEW.city_id, NEW.city_name, NEW.population, ctry_id);
+     else
+         INSERT INTO city_table(city_name, population, country_id)
+             VALUES(NEW.city_name, NEW.population, ctry_id)
+             RETURNING city_id INTO NEW.city_id;
+     end if;
+
+     RETURN NEW;
+ end;
+ $$;
+
+  CREATE TRIGGER city_insert_trig INSTEAD OF INSERT ON city_view
+ FOR EACH ROW EXECUTE PROCEDURE city_insert();
+
+ CREATE OR REPLACE FUNCTION city_delete() RETURNS trigger LANGUAGE plpgsql AS $$
+ begin
+     DELETE FROM city_table WHERE city_id = OLD.city_id;
+     if NOT FOUND then RETURN NULL; end if;
+     RETURN OLD;
+ end;
+ $$;
+
+ CREATE TRIGGER city_delete_trig INSTEAD OF DELETE ON city_view
+ FOR EACH ROW EXECUTE PROCEDURE city_delete();
+
+ CREATE OR REPLACE FUNCTION city_update() RETURNS trigger LANGUAGE plpgsql AS $$
+ declare
+     ctry_id int;
+ begin
+     if NEW.country_name IS DISTINCT FROM OLD.country_name then
+         SELECT country_id, continent INTO ctry_id, NEW.continent
+             FROM country_table WHERE country_name = NEW.country_name;
+         if NOT FOUND then
+             raise exception 'No such country: "%"', NEW.country_name;
+         end if;
+
+         UPDATE city_table SET city_name = NEW.city_name,
+                               population = NEW.population,
+                               country_id = ctry_id
+             WHERE city_id = OLD.city_id;
+     else
+         UPDATE city_table SET city_name = NEW.city_name,
+                               population = NEW.population
+             WHERE city_id = OLD.city_id;
+         NEW.continent := OLD.continent;
+     end if;
+
+     if NOT FOUND then RETURN NULL; end if;
+     RETURN NEW;
+ end;
+ $$;
+
+ CREATE TRIGGER city_update_trig INSTEAD OF UPDATE ON city_view
+ FOR EACH ROW EXECUTE PROCEDURE city_update();
+
+-- INSERT .. RETURNING
+ INSERT INTO city_view(city_name) VALUES('Tokyo') RETURNING *;
+ INSERT INTO city_view(city_name, population) VALUES('London', 7556900) RETURNING *;
+ INSERT INTO city_view(city_name, country_name) VALUES('Washington DC', 'USA') RETURNING *;
+ INSERT INTO city_view(city_id, city_name) VALUES(123456, 'New York') RETURNING *;
+ INSERT INTO city_view VALUES(234567, 'Birmingham', 1016800, 'UK', 'EU') RETURNING *;
+
+ -- UPDATE .. RETURNING
+ UPDATE city_view SET country_name = 'Japon' WHERE city_name = 'Tokyo'; -- error
+ UPDATE city_view SET country_name = 'Japan' WHERE city_name = 'Takyo'; -- no match
+ UPDATE city_view SET country_name = 'Japan' WHERE city_name = 'Tokyo' RETURNING *; -- OK
+
+ UPDATE city_view SET population = 13010279 WHERE city_name = 'Tokyo' RETURNING *;
+ UPDATE city_view SET country_name = 'UK' WHERE city_name = 'New York' RETURNING *;
+ UPDATE city_view SET country_name = 'USA', population = 8391881 WHERE city_name = 'New York' RETURNING *;
+ UPDATE city_view SET continent = 'EU' WHERE continent = 'Europe' RETURNING *;
+ UPDATE city_view v1 SET country_name = v2.country_name FROM city_view v2
+     WHERE v2.city_name = 'Birmingham' AND v1.city_name = 'London' RETURNING *;
+
+ -- DELETE .. RETURNING
+ DELETE FROM city_view WHERE city_name = 'Birmingham' RETURNING *;
+
+
+DROP extension pg_tde CASCADE;

src/access/pg_tdeam.c

@@ -76,6 +76,7 @@
 #include "utils/relcache.h"
 #include "utils/snapmgr.h"
 #include "utils/spccache.h"
+#include "utils/memutils.h"
 
 
 static HeapTuple pg_tde_prepare_insert(Relation relation, HeapTuple tup,
@@ -3010,6 +3011,7 @@ pg_tde_update(Relation relation, ItemPointer otid, HeapTuple newtup,
 	Bitmapset  *modified_attrs;
 	ItemId		lp;
 	HeapTupleData oldtup;
+	HeapTupleData oldtup2;
 	HeapTuple	heaptup;
 	HeapTuple	old_key_tuple = NULL;
 	bool		old_key_copied = false;
@@ -3112,9 +3114,24 @@ pg_tde_update(Relation relation, ItemPointer otid, HeapTuple newtup,
 	oldtup.t_len = ItemIdGetLength(lp);
 	oldtup.t_self = *otid;
 	/* decrypt the old tuple */
-	PG_TDE_DECRYPT_TUPLE(BufferGetBlockNumber(buffer), page, &oldtup, &oldtup,
+	{
+		MemoryContext oldContext;
+		char* new_ptr = NULL;
+		oldContext = MemoryContextSwitchTo(CurTransactionContext);
+		new_ptr = palloc(oldtup.t_len);
+		MemoryContextSwitchTo(oldContext);
+		memcpy(new_ptr, oldtup.t_data, oldtup.t_len);
+		// only neccessary field
+		oldtup2.t_data = (HeapTupleHeader)new_ptr;
+	}
+	PG_TDE_DECRYPT_TUPLE(BufferGetBlockNumber(buffer), page, &oldtup, &oldtup2,
 							GetRelationKeys(relation->rd_locator));
 
+	// change field in oldtup now.
+	// We can't do it before, as PG_TDE_DECRYPT_TUPLE uses t_data address in 
+	// calculations
+	oldtup.t_data = oldtup2.t_data;
+
 	/* the new tuple is ready, except for this: */
 	newtup->t_tableOid = RelationGetRelid(relation);