Re: Strange TRIGGER failure with FOR ... IN ... LOOP ... INSERT

From: Christopher BROWN <brown(at)reflexe(dot)fr>
To: Charles Clavadetscher <clavadetscher(at)swisspug(dot)org>
Cc: pgsql-general(at)postgresql(dot)org
Subject: Re: Strange TRIGGER failure with FOR ... IN ... LOOP ... INSERT
Date: 2015-08-27 13:24:41
Message-ID: CAHL_zcPSDJ1titRW+temGdwLup6RWPitFLY5+TGY3JN04=4SFw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-general

Hello Charles,

Your first suggestion effectively matches what I did as a workaround, with
the advantage that your suggestion explicitly orders the columns and has an
explanation attached... and your second suggestion seems to match what I
was originally trying to do.

I was, perhaps naively, trying to select only the columns I needed, trying
to avoid "SELECT *" as it's "considered harmful"... ; maybe in this context
I don't need to worry..?

Getting back to your second suggestion, it would seem that declaring "r
RECORD" is a very good solution, because it also has the advantage of
future-proofing function code against changes to the declared
tablename%rowtype (no column suddenly becomes null due to structural
changes). Would that be a reasonable assertion? It would in that case
seem that trying to strongly-type the variable does more harm than good in
this context.

As a side note, I was also able to eliminate returning the "id" column
value too, as a result of declaring "r" as "RECORD". I originally added it
there to avoid an error message complaining about "r cannot be converted to
an integer" (or something like that), but I've figured why I got that
message too now, based on your explanation.

Thanks,
Christopher

On 27 August 2015 at 15:01, Charles Clavadetscher <
clavadetscher(at)swisspug(dot)org> wrote:

> Another possibility is
>
>
>
> CREATE OR REPLACE FUNCTION init_store_ldap_profiles() RETURNS TRIGGER AS $$
>
> DECLARE
>
> r RECORD;
>
> 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;
>
>
>
>
>
> *From:* pgsql-general-owner(at)postgresql(dot)org [mailto:
> pgsql-general-owner(at)postgresql(dot)org] *On Behalf Of *Charles Clavadetscher
> *Sent:* Donnerstag, 27. August 2015 14:57
> *To:* pgsql-general(at)postgresql(dot)org
> *Subject:* Re: [GENERAL] Strange TRIGGER failure with FOR ... IN ... LOOP
> ... INSERT
>
>
>
> Hello
>
>
>
> You declare your variable r as of type
> application.store_ldap_profile_defaults%rowtype, but then select only 4 of
> the 5 fields of the table to put in there. The last one (happens to be
> access_mode is then null).
>
> The structures don’t match. That may explain this behaviour.
>
>
>
> This works:
>
>
>
> 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, format,
> 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;
>
>
>
> Bye
>
> Charles
>
>
>
> *From:* pgsql-general-owner(at)postgresql(dot)org [
> mailto:pgsql-general-owner(at)postgresql(dot)org
> <pgsql-general-owner(at)postgresql(dot)org>] *On Behalf Of *Christopher BROWN
> *Sent:* Donnerstag, 27. August 2015 13:50
> *To:* pgsql-general(at)postgresql(dot)org
> *Subject:* [GENERAL] Strange TRIGGER failure with FOR ... IN ... LOOP ...
> INSERT
>
>
>
> 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 ?
>
>
>
> 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;
>
>
>

In response to

Browse pgsql-general by date

  From Date Subject
Next Message Adrian Klaver 2015-08-27 13:25:37 Re: Strange TRIGGER failure with FOR ... IN ... LOOP ... INSERT
Previous Message Adrian Klaver 2015-08-27 13:06:22 Re: Strange TRIGGER failure with FOR ... IN ... LOOP ... INSERT