[prev in list] [next in list] [prev in thread] [next in thread]
List: postgresql-general
Subject: [HACKERS] Fix bug in handling of dropped columns in pltcl triggers
From: Jim Nasby <Jim.Nasby () BlueTreble ! com>
Date: 2016-10-31 19:17:37
Message-ID: b2de8258-c4c0-1cb8-7b97-e8538e5c975c () BlueTreble ! com
[Download RAW message or body]
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
["pltcl_trigger_return.patch" (text/plain)]
commit 8f88fda52751f88aca4786f45d3d7f16a5343fc0
Author: Jim Nasby <Jim.Nasby@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
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic