Hi,
I was investigating the cases when the system attributes are accessed
beyond the scans. After investigating set_plan_references(), I thought
that we never access system attributes beyond scans. This lead me to
assume that EEOP_INNER/OUTER_SYSVAR are not needed since we do not
access system attributes from an inner or outer slot. I removed the
defintions and code using those and ran regression. All the tests
passed. So, I was about to conclude that my assumption is correct. But
then looking at TriggerEnabled() I realised that we also (ab?)use
INNER/OUTER Vars for OLD/NEW tuples for trigger condition. If the WHEN
condition in CREATE TRIGGER command refers to a system attribute, we
will end up having INNER/OUTER var refering a system attribute, thus
exercising code for EEOP_INNER/OUTER_SYSVAR.
Here's patch containing a testcase exercizing that code using
EEOP_INNER/OUTER_SYSVAR.
0001 is the actual testcase and the output
0002 adds warnings where EEOP_OUTER/INNER_SYSVAR are used. This patch
is not for commit. It is only to make sure that the code gets
exercised by the test. When both the patches are applied, the test
shows those warnings.
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
From d08457c3b7bfaba7fa66dc053e01d12184bf427d Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.ba...@enterprisedb.com>
Date: Thu, 10 May 2018 10:58:56 +0530
Subject: [PATCH 01/10] Testcase accessing system columns from a trigger
function.
The column references in OLD and NEW tuples in a trigger condition get
translated to INNER and OUTER Var nodes. If the trigger condition is
referencing any system variable, those references would translate into
EEOP_INNER/OUTER_SYSVAR opcodes. There's no test exercizing this code
right now. Hence adding one.
Ashutosh Bapat
---
src/test/regress/expected/triggers.out | 17 +++++++++++++++++
src/test/regress/sql/triggers.sql | 17 +++++++++++++++++
2 files changed, 34 insertions(+)
diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out
index 1f8caef..732bd2b 100644
--- a/src/test/regress/expected/triggers.out
+++ b/src/test/regress/expected/triggers.out
@@ -2788,3 +2788,20 @@ drop table self_ref;
drop function dump_insert();
drop function dump_update();
drop function dump_delete();
+-- Test triggers accessing system columns
+create table sys_col_table(a int) with oids;
+insert into sys_col_table values (1);
+create function sys_col_trig_func() returns trigger as $$
+begin
+ raise notice 'oid of the row did not change';
+ return new;
+end; $$ language plpgsql;
+create trigger sys_col_trig after update on sys_col_table
+ for each row
+ when (new.oid = old.oid)
+ execute procedure sys_col_trig_func();
+update sys_col_table set a = a + 1;
+NOTICE: oid of the row did not change
+drop trigger sys_col_trig on sys_col_table;
+drop function sys_col_trig_func;
+drop table sys_col_table;
diff --git a/src/test/regress/sql/triggers.sql b/src/test/regress/sql/triggers.sql
index 7cfa5fd..4715090 100644
--- a/src/test/regress/sql/triggers.sql
+++ b/src/test/regress/sql/triggers.sql
@@ -2129,3 +2129,20 @@ drop table self_ref;
drop function dump_insert();
drop function dump_update();
drop function dump_delete();
+
+-- Test triggers accessing system columns
+create table sys_col_table(a int) with oids;
+insert into sys_col_table values (1);
+create function sys_col_trig_func() returns trigger as $$
+begin
+ raise notice 'oid of the row did not change';
+ return new;
+end; $$ language plpgsql;
+create trigger sys_col_trig after update on sys_col_table
+ for each row
+ when (new.oid = old.oid)
+ execute procedure sys_col_trig_func();
+update sys_col_table set a = a + 1;
+drop trigger sys_col_trig on sys_col_table;
+drop function sys_col_trig_func;
+drop table sys_col_table;
--
1.7.9.5
From c2f164213ae90b29ca155ce72526e5c750b4c15d Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.ba...@enterprisedb.com>
Date: Thu, 10 May 2018 10:58:00 +0530
Subject: [PATCH 02/10] Add a warning message when creating
EEOP_INNER/OUTER_SYSVAR opcode.
Should be removed later.
---
src/backend/executor/execExpr.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
index e284fd7..0b91f3c 100644
--- a/src/backend/executor/execExpr.c
+++ b/src/backend/executor/execExpr.c
@@ -673,9 +673,11 @@ ExecInitExprRec(Expr *node, ExprState *state,
{
case INNER_VAR:
scratch.opcode = EEOP_INNER_SYSVAR;
+ elog(WARNING, "accessing inner sysvar");
break;
case OUTER_VAR:
scratch.opcode = EEOP_OUTER_SYSVAR;
+ elog(WARNING, "accessing outer sysvar");
break;
/* INDEX_VAR is handled by default case */
--
1.7.9.5
