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/