2009/12/4 Dean Rasheed <dean.a.rasheed@googlemail.com>:
> With CVS HEAD...
>
> create table foo (a int);
>
> create or replace function foo_trig_fn() returns trigger as $$
> begin
> raise notice 'In trigger: added %', new.ctid;
> return new;
> end
> $$ language plpgsql;
>
> create trigger foo_trig after insert on foo
> for each row execute procedure foo_trig_fn();
>
> insert into foo values(1);
>
> ERROR: attribute number -1 exceeds number of columns 1
>
I started thinking about this again, and it does indeed seem to be the commit
http://archives.postgresql.org/pgsql-committers/2009-11/msg00035.php which
causes this. Specifically, the change
* Avoid unnecessary scanner-driven lookups of plpgsql variables in places where it's not needed, which is actually
mostof the time; we do not need it in DECLARE sections nor in text that is a SQL query or expression.
So read_sql_construct() now disables plpgsql variable lookups in
plpgsql_parse_dblword(), and old.foo/new.foo are compiled into FieldSelect
nodes, where they used to be record field param nodes, which is a problem for
ExecEvalFieldSelect() if foo is a system attribute.
How much do you really save by avoiding the plpgsql variable lookups in this
case? Is this just trading compilation time for execution time? In theory the
new code will be slower to execute because ExecEvalFieldSelect() goes through
ExecEvalParam() to get (a copy of) the whole record in order to extract the
required field, whereas the old code just calls ExecEvalParam() with dtype of
PLPGSQL_DTYPE_RECFIELD to retrieve the field directly. So perhaps
plpgsql_parse_dblword() should always just do the variable lookups.
Thoughts?
Regards,
Dean
