Issue with a control structure in pl/pgsql script loop

Question

I made a little pl/pgsql script to rename some sequences (prefixes adding) and set their schema to 'public'. However I don't understand why but my 'ELSE' instructions are executed only once in the loop, which is not logical because I have many rows whose value of 'nspname' is other than 'docuprocess' :

CREATE OR REPLACE FUNCTION move_schemas_to_public(target_schemas text[]) RETURNS integer AS $procedure$
DECLARE
    rec RECORD;
    sql text;
    newname text;
    nbreq integer := 0;
    tabsize integer := array_length(target_schemas, 1);
    i integer := 1;
    debug boolean := false;
BEGIN

    -- [...]

    FOR rec in
        select nspname, c.relname
        from pg_class c 
        inner join pg_namespace ns 
        on (c.relnamespace = ns.oid) 
        where c.relkind = 'S'
        and ns.nspname = any(target_schemas)
        order by 1, 2
    LOOP
        IF rec.nspname = 'docuprocess' THEN 
            newname := rec.relname;
        ELSE
            -- Why these instructions are executed only once : -----
            newname := rec.nspname||'_'||rec.relname;
            sql := 'ALTER SEQUENCE '||rec.nspname||'.'||rec.relname||' RENAME TO '||newname;
            RAISE NOTICE '%', sql;
            IF debug is not true THEN
                EXECUTE sql;
            END IF;
            nbreq := nbreq + 1;
            --------------------------------------------------------
        END IF;

        sql := 'ALTER SEQUENCE '||rec.nspname||'.'||newname||' SET SCHEMA public';
        RAISE NOTICE '%', sql;
        IF debug is not true THEN
            EXECUTE sql;
        END IF;
        nbreq := nbreq + 1;

    END LOOP;

    -- [...]

    RETURN nbreq;
END;

select move_schemas_to_public(
    -- schemas list
    ARRAY[
        'docufacture',
        'docuprocess',
        'formulaire',
        'notification'
    ]
);

Here is the result for the loop's SQL query :

        [nspname];[relname]

    "docufacture";"exportdoc_idexportdoc_seq"  
    "docufacture";"tableau_idcolonne_seq" 
    "docuprocess";"dp_action_champsdocuged_seq" 
    "docuprocess";"dp_action_commentaire_seq" 
    "docuprocess";"dp_action_docuged_seq" 
    "docuprocess";"dp_action_email_id_seq" 
    "docuprocess";"dp_action_formulaire_seq" 
    "docuprocess";"dp_action_id_seq" 
    "docuprocess";"dp_action_imprimer_id_seq" 
    "docuprocess";"dp_action_lancer_processus_id_seq"
    "docuprocess";"dp_action_lancer_programme_id_seq" 
    "docuprocess";"dp_action_seq" 
    "docuprocess";"dp_action_transfert_fichier_id_seq" 
    "docuprocess";"dp_deroulement_etape_seq" 
    "docuprocess";"dp_deroulement_processus_seq" 
    "docuprocess";"dp_etape_seq" 
    "docuprocess";"dp_indisponibilite_seq" 
    "docuprocess";"dp_intervenant_seq" 
    "docuprocess";"dp_processus_seq" 
    "docuprocess";"dp_type_action_seq" 
    "formulaire";"champ_id_seq" 
    "formulaire";"fond_id_seq" 
    "formulaire";"formulaire_id_seq" 
    "formulaire";"modele_id_seq" 
    "notification";"notification_id_seq"

Thanks in advance for precious help.

I'm not sure, what you're asking, but only `LOOP` body executed multiple times, while `IF`/`ELSE` branches are not. — pozs, Jul 10 '14 at 12:37
It's almost that. In fact, the instructions present in ELSE branch only is executed once and the loop break then. However, I just realizing that when my 'debug' variable is fixed on true, the instructions are correctly executed, as if the 'EXECUTE' instruction breaks the loop. — Antonin, Jul 10 '14 at 12:57
I don't see your version of Postgres, which should *always* be included with such questions. Also, even more importantly, you did not include the function header, which is integral part of the function. Always post the *complete* function. The header is probably the crucial part here. — Erwin Brandstetter, Jul 10 '14 at 14:01
I use 8.4 version (postgreSQL). Ok, I'll edit my post to include header. — Antonin, Jul 10 '14 at 14:20

Antonin · Accepted Answer · 2014-07-11T08:53:24.450

I finally found the source of the problem! In the beginning of my function (masked part "[...]"), I have a loop which rename tables in schemas passed as parameters, and move these tables to schema 'public'. At this time, sequences owned by tables present in 'docufacture' and 'notification' schemas are automatically moved into public schema.

So, I just have to rename sequences for these schemas, not moving them. However I don't really understand why sequences of 'docuprocess' and 'formulaire' aren't moved in the same manner!

Indeed, if I try to execute the following request after tables shifting...

ALTER SEQUENCE docufacture.exportdoc_idexportdoc_seq RENAME TO docufacture_exportdoc_idexportdoc_seq

...I got this error :

ERROR:  relation "docufacture.exportdoc_idexportdoc_seq" does not exist

...because "exportdoc_idexportdoc_seq" has been moved to public schema.

And if I I try to execute the following request after tables shifting...

ALTER SEQUENCE exportdoc_idexportdoc_seq SET SCHEMA public;

...I got this error :

ERROR:  cannot move an owned sequence into another schema

If someone has some explanations about that, it will be really appreciated. Thanks a lot!

EDIT :

So, one solution is to proceed in 3 steps :

Rename all sequences
Move tables
Move remaining sequences

Here is the code :

CREATE OR REPLACE FUNCTION move_schemas_to_public(target_schemas text[]) RETURNS integer AS $procedure$
DECLARE
    rec RECORD;
    sql text;
    newname text;
    nbreq integer := 0;
    tabsize integer := array_length(target_schemas, 1);
    i integer := 1;
    debug boolean := false;
BEGIN

    SET lc_messages TO 'en_US.UTF-8';   

    -- sequences renamming

    FOR rec in
        select ns.nspname, c.relname
        from pg_class c 
        inner join pg_namespace ns 
        on (c.relnamespace = ns.oid) 
        where c.relkind = 'S'
        and ns.nspname = any(target_schemas)
    LOOP
        IF rec.nspname != 'docuprocess' THEN
            newname := quote_ident(rec.nspname||'_'||rec.relname);
            sql := 'ALTER SEQUENCE '||quote_ident(rec.nspname)||'.'||quote_ident(rec.relname)||' RENAME TO '||newname;
            RAISE NOTICE '%', sql;
            IF debug is not true THEN
                EXECUTE sql;
            END IF;
            nbreq := nbreq + 1;
        END IF;
    END LOOP;

    -- END sequences


    -- tables

    FOR rec in
        SELECT table_schema, table_name
        from information_schema.tables
        where table_type = 'BASE TABLE'
        and table_schema = any(target_schemas)
    LOOP
        IF rec.table_schema = 'docuprocess' THEN
            newname := rec.table_name;
        ELSE
            newname := rec.table_schema||'_'||rec.table_name;
            sql := 'ALTER TABLE '||rec.table_schema||'.'||rec.table_name||' RENAME TO '||newname;
            RAISE NOTICE '%', sql;
            IF debug is not true THEN
                EXECUTE sql;
            END IF;
            nbreq := nbreq + 1;
        END IF;

        sql := 'ALTER TABLE '||rec.table_schema||'.'||newname||' SET SCHEMA public';
        RAISE NOTICE '%', sql;
        IF debug is not true THEN
            EXECUTE sql;
        END IF;
        nbreq := nbreq + 1;

    END LOOP;

    -- END tables


    -- remaining sequences shifting

    FOR rec in
        select ns.nspname, c.relname
        from pg_class c 
        inner join pg_namespace ns 
        on (c.relnamespace = ns.oid) 
        where c.relkind = 'S'
        and ns.nspname = any(target_schemas)
    LOOP
        sql := 'ALTER SEQUENCE '||quote_ident(rec.nspname)||'.'||quote_ident(rec.relname)||' SET SCHEMA public';
        RAISE NOTICE '%', sql;
        IF debug is not true THEN
            EXECUTE sql;
        END IF;
        nbreq := nbreq + 1;
    END LOOP;

    -- END sequences


    -- [...] Move functions, drop empty schemas


    RETURN nbreq;
END;

$procedure$ 
LANGUAGE plpgsql;

select move_schemas_to_public(
    -- schemas list
    ARRAY[
        'docufacture',
        'docuprocess',
        'formulaire',
        'notification'
    ]
);

To finish, I would like to address special thanks to "Erwin Brandstetter" for his advanced help and advices.

Ha, so the problem was hidden in the clipped part. Kind of explains, why we couldn't pin it down. :) Remember to *always* sanitize identifiers when used in dynamic SQL. — Erwin Brandstetter, Jul 12 '14 at 02:42

score 0 · Answer 2 · answered Jul 10 '14 at 12:49

0

There is a space missing in this line after RENAME TO:

sql := 'ALTER SEQUENCE '||rec.nspname||'.'||rec.relname||' RENAME TO '||newname;

As a result, upon the first sequence that is not in schema docuprocess the sql statement is executed and raises an error which aborts the loop.

Note also that you do not have to ORDER BY the rec query because you are evaluating record properties in the loop and not using the ordering of the qualifying records.

answered Jul 10 '14 at 12:49

Patrick

29,357
6
62
90

Well seen Patrick. Sadly, it was just a typo on my post... :/ I edit that just now. – Antonin Jul 10 '14 at 13:02
Here is the last notice writen in output console before loop abort : ALTER SEQUENCE formulaire.formulaire_fond_id_seq SET SCHEMA public – Antonin Jul 10 '14 at 13:14
@Ant0nin: OK, so the loop aborts? There should be an error message (which should be in your question). Might be that the role you are running this with a role that just does not have the privileges for some of your schemas / sequences and bails ... – Erwin Brandstetter Jul 10 '14 at 14:35
I suppose it aborts. I tried with another role and same problem. What is very weird is that I have no blocking error, the script continue until the end of the function, all other SQL requests of the function are performed and work. The only error message that I have is in French but if I translate it could something like "Result of request consisting of a 1 line canceled". – Antonin Jul 10 '14 at 14:56
@Ant0nin: To get English error messages, append `SET lc_messages = 'C'` to the function header (or anywhere in your session). Like: `LANGUAGE plpgsql lc_messages = 'C'`. – Erwin Brandstetter Jul 10 '14 at 15:01

score 0 · Answer 3 · edited May 23 '17 at 12:05

Naming collision?

I notice you did not table-qualify nspname in the SELECT list of your SQL statement:

select nspname, c.relname
from pg_class c 
inner join pg_namespace ns 
on (c.relnamespace = ns.oid) 
where c.relkind = 'S'
and ns.nspname = any(target_schemas)
order by 1, 2

While the one in the WHERE clause is table table-qualified.

You did not provide the function header, but if there is a variable or function parameter of the same name nspname it takes precedence. Then you would have the constant value of that variable in the result of your query, which would explain the observed behaviour.

It's a bad idea to allow such naming collisions in the first place. I make it a happit to prepend variables and parameters with _. Like _nspname.
But if you have such collisions, you need to be unambiguous in your SQL statements and always table-qualify ambiguous column names. Or, for simplicity, all column names.

select ns.nspname, c.relname
from pg_class c ...

Similar case:

Postgres Function NULL value for row that references NEW

Privileges?

If that's not the problem, it might be a case of missing privileges. Per pg 8.4 documentation:

You must own the sequence to use ALTER SEQUENCE. To change a sequence's schema, you must also have CREATE privilege on the new schema.

There should be an error message! Check your db logs ...

Always sanitize identifiers

And you need to sanitize identifiers when used in dynamic SQL:

...

  newname := quote_ident(rec.relname);
    ELSE

...
        newname := quote_ident(rec.nspname||'_'||rec.relname);


sql := 'ALTER SEQUENCE ' || quote_ident(rec.nspname) || '.' || quote_ident(rec.relname)
    || ' RENAME TO ' || newname;

Etc. - in all instances. Else, your statements break if any of your identifiers are non-standard (mixed case, reserved words, space, ...). Even allows SQL injection. (!)
Careful not to apply quote_ident() before you build the new name.

Postgres 8.4 is somewhat limited in this regard. Version 9.1 introduces format(). More details here:

Table name as a PostgreSQL function parameter

It may be time to start thinking about an upgrade to a current version.

Maximum length of identifiers

Finally, your identifiers are getting rather long. Bear in mind, the typical maximum length is 63 bytes:

Thanks for your advice Erwin. It's always good to learn something. It doesn't solve my problem by the way. — Antonin, Jul 10 '14 at 14:17
@Ant0nin: Please post your complete function, then. What you describe *very much* smells like a case of the above. — Erwin Brandstetter, Jul 10 '14 at 14:22
Done. However, I removed some parts of the function to avoid mess. — Antonin, Jul 10 '14 at 14:26
All sequences are owned by postgres user. Even if I use the postgres role, nothing changes. — Antonin, Jul 10 '14 at 15:17
I didn't know quote_* functions. Thanks for the security advice. I'll be more careful about that. — Antonin, Jul 10 '14 at 15:19
I can't upgrade to 8.4 for retro-compatibility reasons, I working on a application which have several years old. — Antonin, Jul 10 '14 at 15:26
@Ant0nin: You'll *have to* upgrade soon enough. [Postgres 8.4 is reaching EOL this month (July 2014)](http://www.postgresql.org/support/versioning/). — Erwin Brandstetter, Jul 10 '14 at 15:29

Issue with a control structure in pl/pgsql script loop

3 Answers3

Naming collision?

Privileges?

Always sanitize identifiers

Maximum length of identifiers