Обсуждение: Strange TRIGGER failure with FOR ... IN ... LOOP ... INSERT

On 08/27/2015 04:49 AM, Christopher BROWN wrote:
> Hello,
>
> I'm new to this list but have been using PostgreSQL for a moment.  I've
> encountered an error using PostgreSQL 9.4.4 which can be reproduced
> using the SQL below.
>
> The trigger "init_store_ldap_profiles_trigger" fails if the function
> "init_store_ldap_profiles()" is written as below.  If I rewrite it to
> use "SELECT * FROM ...", instead of "SELECT id, ref_ldap_department,
> ref_ldap_title, access_mode FROM ...", it works.
>
> This is the error I get:
> ERROR: null value in column "access_mode" violates not-null constraint
>    Detail: Failing row contains (1, 2015-08-27 13:37:24.306883,
> 2015-08-27 13:37:24.306883, 1, 1, 1, null).
>    Where: SQL statement "INSERT INTO application.store_ldap_profile
> (ref_store, ref_ldap_department, ref_ldap_title, access_mode) VALUES
> (NEW.id, r.ref_ldap_department, r.ref_ldap_title, r.access_mode)"
> PL/pgSQL function init_store_ldap_profiles() line 8 at SQL statement


I think you have a bigger problem. The failing row has 7 values where
you are sending 4 values. Given the 2 defaults for time that still only
adds up to 6. Also I not sure how you can get a NULL for access_mode as
the table you are selecting from store_ldap_profile_defaults, has
access_mode VARCHAR(1) NOT NULL CHECK (access_mode IN ('R', 'W')),

>
> It seems that for some reason, the column
> "store_ldap_profile_defaults.access_mode" appears to be NULL when
> referred to using r.access_mode (r being the declared %ROWTYPE).  I can
> modify the WHERE clause to add a dummy condition on "access_mode", and
> that works (as in, it doesn't solve my problem but the column value is
> visible to the WHERE clause).
>
> Is this a bug or can I fix this in my SQL ?
>
> Thanks,
> Christopher
>
> Here's the SQL :
>
>
> CREATE SCHEMA application;
> SET search_path TO application;
>
> CREATE TABLE IF NOT EXISTS store (
> id SERIAL PRIMARY KEY,
> ctime TIMESTAMP NOT NULL DEFAULT now(),
> mtime TIMESTAMP NOT NULL DEFAULT now(),
> is_archived NUMERIC(1) CHECK (is_archived IN (1,0)) DEFAULT 0,
> name VARCHAR(200) NOT NULL CHECK (length(name) > 0),
> hrcompany VARCHAR(200) NOT NULL CHECK (length(hrcompany) > 0),
> hrsite VARCHAR(200) NOT NULL CHECK (length(hrsite) > 0),
> format VARCHAR(1) NOT NULL CHECK (format IN ('H', 'S')),
> UNIQUE (hrcompany, hrsite)
> );
>
> CREATE INDEX ON store (mtime);
> CREATE INDEX ON store (is_archived);
> CREATE INDEX ON store (format);
>
>
> CREATE TABLE IF NOT EXISTS ldap_department (
> id SERIAL PRIMARY KEY,
> ctime TIMESTAMP NOT NULL DEFAULT now(),
> mtime TIMESTAMP NOT NULL DEFAULT now(),
> code VARCHAR(20) NOT NULL CHECK (code ~ '[0-9]+'),
> label VARCHAR(200) NOT NULL CHECK (length(label) > 0),
> UNIQUE(code)
> );
>
> CREATE INDEX ON ldap_department (mtime);
>
>
> CREATE TABLE IF NOT EXISTS ldap_title (
> id SERIAL PRIMARY KEY,
> ctime TIMESTAMP NOT NULL DEFAULT now(),
> mtime TIMESTAMP NOT NULL DEFAULT now(),
> code VARCHAR(20) NOT NULL CHECK (code ~ '[0-9]+'),
> label VARCHAR(200) NOT NULL CHECK (length(label) > 0),
> UNIQUE(code)
> );
>
> CREATE INDEX ON ldap_title (mtime);
>
>
> CREATE TABLE IF NOT EXISTS store_ldap_profile_defaults (
> id SERIAL PRIMARY KEY,
> ref_ldap_department INTEGER NOT NULL,
> ref_ldap_title INTEGER NOT NULL,
> format VARCHAR(1) NOT NULL CHECK (format IN ('H', 'S')),
> access_mode VARCHAR(1) NOT NULL CHECK (access_mode IN ('R', 'W')),
> FOREIGN KEY (ref_ldap_department) REFERENCES ldap_department (id) ON
> DELETE CASCADE,
> FOREIGN KEY (ref_ldap_title) REFERENCES ldap_title (id) ON DELETE CASCADE,
> UNIQUE (ref_ldap_department, ref_ldap_title, format)
> );
>
> CREATE INDEX ON store_ldap_profile_defaults (format);
> CREATE INDEX ON store_ldap_profile_defaults (access_mode);
>
>
> CREATE TABLE IF NOT EXISTS store_ldap_profile (
> id SERIAL PRIMARY KEY,
> ctime TIMESTAMP NOT NULL DEFAULT now(),
> mtime TIMESTAMP NOT NULL DEFAULT now(),
> ref_store INTEGER NOT NULL,
> ref_ldap_department INTEGER NOT NULL,
> ref_ldap_title INTEGER NOT NULL,
> access_mode VARCHAR(1) NOT NULL CHECK (access_mode IN ('R', 'W')),
> FOREIGN KEY (ref_store) REFERENCES store (id) ON DELETE RESTRICT,
> FOREIGN KEY (ref_ldap_department) REFERENCES ldap_department (id) ON
> DELETE CASCADE,
> FOREIGN KEY (ref_ldap_title) REFERENCES ldap_title (id) ON DELETE CASCADE,
> UNIQUE (ref_store, ref_ldap_department, ref_ldap_title)
> );
>
> CREATE INDEX ON store_ldap_profile (mtime);
> CREATE INDEX ON store_ldap_profile (ref_store);
>
>
> DROP TRIGGER IF EXISTS touch_store_ldap_profile_trigger
> ON application.store_ldap_profile;
>
> CREATE OR REPLACE FUNCTION touch_store_ldap_profile() RETURNS TRIGGER AS $$
> BEGIN
> UPDATE application.store SET mtime = now() WHERE id = NEW.ref_store;
> RETURN NEW;
> END; $$
> LANGUAGE plpgsql VOLATILE;
>
> CREATE TRIGGER touch_store_ldap_profile_trigger
> AFTER INSERT OR UPDATE ON application.store_ldap_profile
> FOR EACH ROW EXECUTE PROCEDURE touch_store_ldap_profile();
>
>
> DROP TRIGGER IF EXISTS init_store_ldap_profiles_trigger
> ON application.store;
>
> CREATE OR REPLACE FUNCTION init_store_ldap_profiles() RETURNS TRIGGER AS $$
> DECLARE
> r application.store_ldap_profile_defaults%rowtype;
> BEGIN
> FOR r IN
> SELECT id, ref_ldap_department, ref_ldap_title, access_mode FROM
> application.store_ldap_profile_defaults WHERE format = NEW.format
> LOOP
> INSERT INTO application.store_ldap_profile (ref_store,
> ref_ldap_department, ref_ldap_title, access_mode) VALUES (NEW.id,
> r.ref_ldap_department, r.ref_ldap_title, r.access_mode);
> END LOOP;
> RETURN NEW;
> END; $$
> LANGUAGE plpgsql VOLATILE;
>
> CREATE TRIGGER init_store_ldap_profiles_trigger
> AFTER INSERT ON application.store
> FOR EACH ROW EXECUTE PROCEDURE init_store_ldap_profiles();
>
> INSERT INTO ldap_department (code, label) VALUES
> ('03000', 'CAISSES');
>
> INSERT INTO ldap_title (code, label) VALUES
> ('814', 'MANAGER SERV CAISSES'),
> ('837', 'RESPONSABLE SERVICE CAISSES');
>
> INSERT INTO store_ldap_profile_defaults (ref_ldap_department,
> ref_ldap_title, format, access_mode) VALUES
> ((SELECT id FROM ldap_department WHERE code = '03000' LIMIT 1), (SELECT
> id FROM ldap_title WHERE code = '814' LIMIT 1), 'H', 'R'),
> ((SELECT id FROM ldap_department WHERE code = '03000' LIMIT 1), (SELECT
> id FROM ldap_title WHERE code = '837' LIMIT 1), 'H', 'W');
>
>
>
> --SET search_path TO "$user",public;
>


--
Adrian Klaver
adrian.klaver@aklaver.com

On 08/27/2015 04:49 AM, Christopher BROWN wrote:
> Hello,
>
> I'm new to this list but have been using PostgreSQL for a moment.  I've
> encountered an error using PostgreSQL 9.4.4 which can be reproduced
> using the SQL below.
>
> The trigger "init_store_ldap_profiles_trigger" fails if the function
> "init_store_ldap_profiles()" is written as below.  If I rewrite it to
> use "SELECT * FROM ...", instead of "SELECT id, ref_ldap_department,
> ref_ldap_title, access_mode FROM ...", it works.
>
> This is the error I get:
> ERROR: null value in column "access_mode" violates not-null constraint
>    Detail: Failing row contains (1, 2015-08-27 13:37:24.306883,
> 2015-08-27 13:37:24.306883, 1, 1, 1, null).
>    Where: SQL statement "INSERT INTO application.store_ldap_profile
> (ref_store, ref_ldap_department, ref_ldap_title, access_mode) VALUES
> (NEW.id, r.ref_ldap_department, r.ref_ldap_title, r.access_mode)"
> PL/pgSQL function init_store_ldap_profiles() line 8 at SQL statement
>
> It seems that for some reason, the column
> "store_ldap_profile_defaults.access_mode" appears to be NULL when
> referred to using r.access_mode (r being the declared %ROWTYPE).  I can
> modify the WHERE clause to add a dummy condition on "access_mode", and
> that works (as in, it doesn't solve my problem but the column value is
> visible to the WHERE clause).
>
> Is this a bug or can I fix this in my SQL ?

It is not a bug, see below for more.

>
> Thanks,
> Christopher
>
> Here's the SQL :

> CREATE OR REPLACE FUNCTION init_store_ldap_profiles() RETURNS TRIGGER AS $$
> DECLARE
> r application.store_ldap_profile_defaults%rowtype;

Per Charles's post the ROWTYPE is tripping you up.

http://www.postgresql.org/docs/9.4/interactive/plpgsql-declarations.html#PLPGSQL-DECLARATION-ROWTYPES

"A variable of a composite type is called a row variable (or row-type
variable). Such a variable can hold a whole row of a SELECT or FOR query
result, so long as that query's column set matches the declared type of
the variable. The individual fields of the row value are accessed using
the usual dot notation, for example rowvar.field."

You are selecting one less field then the ROWTYPE declared type, so
access_mode(the extra field in the ROWTYPE) is set to NULL. It works
when you do * because then the query column count matches the ROWTYPE
column count.

So the choices are:

1) Use ROWTYPE and select all the columns

2) Use RECORD, which adapts itself to the columns returned:

http://www.postgresql.org/docs/9.4/interactive/plpgsql-declarations.html#PLPGSQL-DECLARATION-RECORDS


> BEGIN
> FOR r IN
> SELECT id, ref_ldap_department, ref_ldap_title, access_mode FROM
> application.store_ldap_profile_defaults WHERE format = NEW.format
> LOOP
> INSERT INTO application.store_ldap_profile (ref_store,
> ref_ldap_department, ref_ldap_title, access_mode) VALUES (NEW.id,
> r.ref_ldap_department, r.ref_ldap_title, r.access_mode);
> END LOOP;
> RETURN NEW;
> END; $$
> LANGUAGE plpgsql VOLATILE;
>


--
Adrian Klaver
adrian.klaver@aklaver.com

On 08/27/2015 06:33 AM, Christopher BROWN wrote:
> Hello Adrian,
>
> Yep, Charles' explanation helped me understand what was going on.
> Before that, I was as confused as you were (in your first reply) about
> how access_mode could be NULL (with the same reasoning).  In any case,
> thanks for your links ; I did try searching the web for the answer
> before posting, but got too many irrelevant results given that I had to
> search using very common terms.

Yeah, I did not get Charles's second post until I sent my second, so it
was redundant.

>
> I've concluded the the RECORD type is the best-fit for my approach.  I
> don't know if it's any faster that using SELECT * with a specific
> %ROWTYPE given that the data doesn't go anywhere outside the function
> body.  I don't know if the order in which columns are returned (by
> either SELECT * or using explicit column names matters when using
> %ROWTYPE), although I'll assume that PostgreSQL is smart enough to match
> things up correctly, if I need to write a function that returns
> instances of any given %ROWTYPE in the future.

I don't know, I have always just used SELECT * as I needed all the
columns anyway.

>
> Thanks again.
> Christopher
>
>



--
Adrian Klaver
adrian.klaver@aklaver.com

Hi

> -----Original Message-----
> From: pgsql-general-owner@postgresql.org [mailto:pgsql-general-
> owner@postgresql.org] On Behalf Of Adrian Klaver
> Sent: Donnerstag, 27. August 2015 15:41
> To: Christopher BROWN <brown@reflexe.fr>
> Cc: pgsql-general@postgresql.org
> Subject: Re: [GENERAL] Strange TRIGGER failure with FOR ... IN ... LOOP ...
> INSERT
>
> On 08/27/2015 06:33 AM, Christopher BROWN wrote:
> > Hello Adrian,
> >
> > Yep, Charles' explanation helped me understand what was going on.
> > Before that, I was as confused as you were (in your first reply) about
> > how access_mode could be NULL (with the same reasoning).  In any case,
> > thanks for your links ; I did try searching the web for the answer
> > before posting, but got too many irrelevant results given that I had to
> > search using very common terms.
>
> Yeah, I did not get Charles's second post until I sent my second, so it
> was redundant.
>
> >
> > I've concluded the the RECORD type is the best-fit for my approach.  I
> > don't know if it's any faster that using SELECT * with a specific
> > %ROWTYPE given that the data doesn't go anywhere outside the function
> > body.  I don't know if the order in which columns are returned (by
> > either SELECT * or using explicit column names matters when using
> > %ROWTYPE), although I'll assume that PostgreSQL is smart enough to match
> > things up correctly, if I need to write a function that returns
> > instances of any given %ROWTYPE in the future.
>
> I don't know, I have always just used SELECT * as I needed all the
> columns anyway.

It is probably a matter of taste, more than best practices. I find the variant with RECORD better, because you only
searchfor the fields that you need. In this case it may make a little difference, but if you happen to have a table
withmany more columns, it would be a waste of resources, unless, as in Adrian's case, you do need all the fields. While
usinga rowtype then SELECT * is guaranteed, IMHO, to return that record's fields in the correct order. Listing the
fieldsexplicitly, as you already noticed, could lead to a maintenance nightmare if anything changes in the table
structure.In both cases you are still not safe against, e.g. changes of column names. But I guess that this is not an
issueso far. 

Bye
Charles

On 08/27/2015 06:33 AM, Christopher BROWN wrote:
> Hello Adrian,
>
> Yep, Charles' explanation helped me understand what was going on.
> Before that, I was as confused as you were (in your first reply) about
> how access_mode could be NULL (with the same reasoning).  In any case,
> thanks for your links ; I did try searching the web for the answer
> before posting, but got too many irrelevant results given that I had to
> search using very common terms.
>
> I've concluded the the RECORD type is the best-fit for my approach.  I
> don't know if it's any faster that using SELECT * with a specific
> %ROWTYPE given that the data doesn't go anywhere outside the function
> body.  I don't know if the order in which columns are returned (by
> either SELECT * or using explicit column names matters when using
> %ROWTYPE), although I'll assume that PostgreSQL is smart enough to match
> things up correctly, if I need to write a function that returns
> instances of any given %ROWTYPE in the future.

Order does matter:

create table rowtype_test(id int, fld_1 varchar, fld_2 varchar);

insert into rowtype_test values (1, 'one', 'two');
insert into rowtype_test values (2, 'three', 'four');

CREATE OR REPLACE FUNCTION row_type_test ( )
  RETURNS void
  LANGUAGE plpgsql

AS $function$
DECLARE
     r rowtype_test%rowtype;
BEGIN
     FOR r IN
         SELECT fld_1, id, fld_2 FROM rowtype_test
     LOOP
         RAISE NOTICE '%', r;
     END LOOP;
     RETURN;
END;
$function$
;


test=> select row_type_test();
ERROR:  invalid input syntax for integer: "one"
CONTEXT:  PL/pgSQL function row_type_test() line 5 at FOR over SELECT rows



>
> Thanks again.
> Christopher
>
>


--
Adrian Klaver
adrian.klaver@aklaver.com