Re: Executing Dynamic DDL

On Wed, May 18, 2005 at 09:07:55AM +0200, Samer Abukhait wrote:
>
> i am trying to execute an 'alter table' statement dynamically.. it
> seems that "execute" only works with DML..

A simple example shows that EXECUTE does indeed work with DDL:

CREATE TABLE foo (col1 integer);

CREATE FUNCTION execute_ddl() RETURNS void AS $$
BEGIN
    EXECUTE 'ALTER TABLE foo ADD col2 timestamp NOT NULL';
    RETURN;
END;
$$ LANGUAGE plpgsql VOLATILE;

\d foo
      Table "public.foo"
 Column |  Type   | Modifiers
--------+---------+-----------
 col1   | integer |

SELECT execute_ddl();

\d foo
                Table "public.foo"
 Column |            Type             | Modifiers
--------+-----------------------------+-----------
 col1   | integer                     |
 col2   | timestamp without time zone | not null

The problem appears to be in the function's logic:

> create or replace function em.process_table (
>     p_table              varchar)
> returns void as $$
> declare
>     v_check      bool;
> begin
>     -- Add Creation TimeStamp column if it is not there.
>     select count (*)
>     into   v_check
>     from   em.all_table_columns
>     where  tablename = p_table
>     and    columnname = 'creation_timestamp';

Count returns a bigint but you assign its value to a boolean.  This
should work if the return value is 0 (false) or 1 (true), which
presumably are the only possible counts in this case, but I probably
wouldn't coerce the value that way.  I'd either use EXISTS or assign
count's value to a bigint.

>     if v_check then
>         execute 'alter table em.' || p_table || ' add creation_timestamp
> timestamp not null';
>     end if;

You're saying that if the column exists (i.e., if v_check is true,
meaning that count returned 1), then add the column; you should be
checking if the column *doesn't* exist (i.e., if v_check is false,
meaning that count returned 0).  Also, it's a good idea to use
quote_ident() when building dynamic queries from data that comes
from outside the function.  And you might want to consider using
timestamp with time zone instead of timestamp.

>     return;
> end;$$ language plpgsql;

A function that has side effects should be declared VOLATILE.  And
if the function requires a non-NULL parameter then it should be
STRICT as well.

BTW, the all_table_columns table appears to duplicate information
already contained in the system catalogs.  Is this a contrived
example or are you really doing that?  Do you have a reason for
doing so?

--
Michael Fuhr
http://www.fuhr.org/~mfuhr/