While reviewing code coverage in pltcl, I uncovered a bug in trigger function return handling. If you returned the munged name of a dropped column, that would silently be ignored. It would be unusual to hit this, since dropped columns end up with names like ".......pg.dropped.2.......", but since that's still a legitimate name for a column silently ignoring it seems rather bogus.

Work sponsored by FlightAware.

https://github.com/postgres/postgres/compare/master...decibel:tcl_dropped
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)   mobile: 512-569-9461
commit 8f88fda52751f88aca4786f45d3d7f16a5343fc0
Author: Jim Nasby <jim.na...@bluetreble.com>
Date:   Mon Oct 31 14:11:43 2016 -0500

    Fix trigger dropped column handling
    Previously, if a trigger returned a column that had been dropped, using the 
munged column name,
    no error would be thrown. Since dropping a column does forcibly overwrite 
the column name, it
    would be very unusual for this to happen in practice, but silently ignoring 
what would otherwise
    be a legitimate column name seemed rather bogus.

diff --git a/src/pl/tcl/expected/pltcl_queries.out 
b/src/pl/tcl/expected/pltcl_queries.out
index 6cb1fdb..c3f33d9 100644
--- a/src/pl/tcl/expected/pltcl_queries.out
+++ b/src/pl/tcl/expected/pltcl_queries.out
@@ -184,6 +184,21 @@ select * from T_pkey2 order by key1 using @<, key2 collate 
"C";
 (4 rows)
 
 -- show dump of trigger data
+insert into test_return values(-1,'1 element array');
+ERROR:  trigger's return list must have even number of elements
+insert into test_return values(-2,'return dropped column');
+ERROR:  unrecognized attribute "........pg.dropped.2........"
+insert into test_return values(-3,'return ctid column');
+ERROR:  cannot set system attribute "ctid"
+insert into test_return values(-10,'do nothing');
+insert into test_return values(1,'good');
+select * from test_return;
+ i |  t   
+---+------
+   | 
+ 1 | good
+(2 rows)
+
 insert into trigger_test values(1,'insert');
 NOTICE:  NEW: {i: 1, v: insert}
 NOTICE:  OLD: {}
diff --git a/src/pl/tcl/expected/pltcl_setup.out 
b/src/pl/tcl/expected/pltcl_setup.out
index e65e9e3..707d77b 100644
--- a/src/pl/tcl/expected/pltcl_setup.out
+++ b/src/pl/tcl/expected/pltcl_setup.out
@@ -89,6 +89,33 @@ FOR EACH ROW EXECUTE PROCEDURE trigger_data(23,'skidoo');
 CREATE TRIGGER show_trigger_data_view_trig
 INSTEAD OF INSERT OR UPDATE OR DELETE ON trigger_test_view
 FOR EACH ROW EXECUTE PROCEDURE trigger_data(24,'skidoo view');
+-- test handling of return
+CREATE TABLE test_return(i int, dropme int, t text);
+ALTER TABLE test_return DROP dropme;
+CREATE FUNCTION trigger_return() RETURNS trigger LANGUAGE pltcl AS $body$
+    switch $NEW(i) {
+        -1 {
+                       return {"single element"}
+               }
+               -2 {
+                       spi_exec "SELECT attname FROM pg_attribute WHERE 
attrelid=$TG_relid AND attisdropped"
+                       set a($attname) 1
+                       return [array get a]
+               }
+               -3 {
+                       return {ctid 1}
+               }
+               -10 {
+                       # Will return nothing
+               }
+               default {
+                       return OK
+               }
+       }
+$body$;
+CREATE TRIGGER test_trigger_return
+BEFORE INSERT ON test_return
+FOR EACH ROW EXECUTE PROCEDURE trigger_return();
 --
 -- Trigger function on every change to T_pkey1
 --
diff --git a/src/pl/tcl/pltcl.c b/src/pl/tcl/pltcl.c
index d236890..5d9a857 100644
--- a/src/pl/tcl/pltcl.c
+++ b/src/pl/tcl/pltcl.c
@@ -1128,7 +1128,9 @@ pltcl_trigger_handler(PG_FUNCTION_ARGS, bool pltrusted)
                         * Get the attribute number
                         
************************************************************/
                        attnum = SPI_fnumber(tupdesc, ret_name);
-                       if (attnum == SPI_ERROR_NOATTRIBUTE)
+                       /* Assume system attributes can't be marked as dropped 
*/
+                       if (attnum == SPI_ERROR_NOATTRIBUTE ||
+                                       (attnum > 0 && tupdesc->attrs[attnum - 
1]->attisdropped))
                                ereport(ERROR,
                                                
(errcode(ERRCODE_UNDEFINED_COLUMN),
                                                 errmsg("unrecognized attribute 
\"%s\"",
@@ -1140,12 +1142,6 @@ pltcl_trigger_handler(PG_FUNCTION_ARGS, bool pltrusted)
                                                                ret_name)));
 
                        
/************************************************************
-                        * Ignore dropped columns
-                        
************************************************************/
-                       if (tupdesc->attrs[attnum - 1]->attisdropped)
-                               continue;
-
-                       
/************************************************************
                         * Lookup the attribute type in the syscache
                         * for the input function
                         
************************************************************/
diff --git a/src/pl/tcl/sql/pltcl_queries.sql b/src/pl/tcl/sql/pltcl_queries.sql
index a0a9619..abc6eb0 100644
--- a/src/pl/tcl/sql/pltcl_queries.sql
+++ b/src/pl/tcl/sql/pltcl_queries.sql
@@ -74,6 +74,13 @@ select * from T_pkey1 order by key1 using @<, key2 collate 
"C";
 select * from T_pkey2 order by key1 using @<, key2 collate "C";
 
 -- show dump of trigger data
+insert into test_return values(-1,'1 element array');
+insert into test_return values(-2,'return dropped column');
+insert into test_return values(-3,'return ctid column');
+insert into test_return values(-10,'do nothing');
+insert into test_return values(1,'good');
+select * from test_return;
+
 insert into trigger_test values(1,'insert');
 
 insert into trigger_test_view values(2,'insert');
diff --git a/src/pl/tcl/sql/pltcl_setup.sql b/src/pl/tcl/sql/pltcl_setup.sql
index 8df65a5..42aef60 100644
--- a/src/pl/tcl/sql/pltcl_setup.sql
+++ b/src/pl/tcl/sql/pltcl_setup.sql
@@ -102,6 +102,34 @@ CREATE TRIGGER show_trigger_data_view_trig
 INSTEAD OF INSERT OR UPDATE OR DELETE ON trigger_test_view
 FOR EACH ROW EXECUTE PROCEDURE trigger_data(24,'skidoo view');
 
+-- test handling of return
+CREATE TABLE test_return(i int, dropme int, t text);
+ALTER TABLE test_return DROP dropme;
+CREATE FUNCTION trigger_return() RETURNS trigger LANGUAGE pltcl AS $body$
+    switch $NEW(i) {
+        -1 {
+                       return {"single element"}
+               }
+               -2 {
+                       spi_exec "SELECT attname FROM pg_attribute WHERE 
attrelid=$TG_relid AND attisdropped"
+                       set a($attname) 1
+                       return [array get a]
+               }
+               -3 {
+                       return {ctid 1}
+               }
+               -10 {
+                       # Will return nothing
+               }
+               default {
+                       return OK
+               }
+       }
+$body$;
+CREATE TRIGGER test_trigger_return
+BEFORE INSERT ON test_return
+FOR EACH ROW EXECUTE PROCEDURE trigger_return();
+
 --
 -- Trigger function on every change to T_pkey1
 --
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to