0

I am trying to create a function and I can't find my error in the following code:

CREATE OR REPLACE FUNCTION qwat_od.fn_label_create_fields(table_name varchar, position boolean = true, rotation boolean = true) RETURNS void AS
$BODY$
    BEGIN
        /* Creates columns */
        EXECUTE 'ALTER TABLE qwat_od.'||table_name||' ADD COLUMN label_1_visible  smallint default 1;           ';
        IF position IS TRUE THEN
            EXECUTE 'ALTER TABLE qwat_od.'||table_name||' ADD COLUMN label_1_x        double precision default null;';
            EXECUTE 'ALTER TABLE qwat_od.'||table_name||' ADD COLUMN label_1_y        double precision default null;';
        END IF;
        IF rotation IS TRUE THEN
            EXECUTE 'ALTER TABLE qwat_od.'||table_name||' ADD COLUMN label_1_rotation double precision default null;';
        END IF;
        EXECUTE 'ALTER TABLE qwat_od.'||table_name||' ADD COLUMN label_1_text     varchar(120);';

        EXECUTE 'ALTER TABLE qwat_od.'||table_name||' ADD COLUMN label_2_visible  smallint default 1;           ';
        IF position IS TRUE THEN
            EXECUTE 'ALTER TABLE qwat_od.'||table_name||' ADD COLUMN label_2_x        double precision default null;';
            EXECUTE 'ALTER TABLE qwat_od.'||table_name||' ADD COLUMN label_2_y        double precision default null;';
        END IF;
        IF rotation IS TRUE THEN
            EXECUTE 'ALTER TABLE qwat_od.'||table_name||' ADD COLUMN label_2_rotation double precision default null;';
        END IF;

        EXECUTE 'ALTER TABLE qwat_od.'||table_name||' ADD COLUMN label_2_text     varchar(120);';
        /* Creates constraints */
        EXECUTE 'ALTER TABLE qwat_od.'||table_name||' ADD CONSTRAINT '||table_name||'_label_1_visible FOREIGN KEY (label_1_visible) REFERENCES qwat_vl.visible(vl_code_int) MATCH FULL; CREATE INDEX fki_'||table_name||'_label_1_visible  ON qwat_od.'||table_name||'(label_1_visible);';
        EXECUTE 'ALTER TABLE qwat_od.'||table_name||' ADD CONSTRAINT '||table_name||'_label_2_visible FOREIGN KEY (label_2_visible) REFERENCES qwat_vl.visible(vl_code_int) MATCH FULL; CREATE INDEX fki_'||table_name||'_label_2_visible  ON qwat_od.'||table_name||'(label_2_visible);';
    END;
$BODY$
LANGUAGE 'plpgsql';

I get this:

ERROR:  syntax error at or near "position"
LINE 4: ...wat_od.fn_label_create_fields(table_name varchar, position b...

Did I do something wrong in the declaration of arguments?

4

1 Answer 1

2

@pozs already provided an explanation for the error you saw.
But there is more. Most importantly, you are wide open to SQL injection.

CREATE OR REPLACE FUNCTION qwat_od.fn_label_create_fields(_tbl text, _position bool = true, _rotation bool = true)
  RETURNS void AS
$func$
BEGIN
    /* Creates columns */
   EXECUTE format('ALTER TABLE qwat_od.%I ADD COLUMN label_1_visible smallint default 1', _tbl);

   IF _position THEN
       EXECUTE format('ALTER TABLE qwat_od.%I
           ADD COLUMN label_1_x double precision default null
         , ADD COLUMN label_1_y double precision default null', _tbl);
   END IF;
   IF _rotation THEN
       EXECUTE format('ALTER TABLE qwat_od.%I ADD COLUMN label_1_rotation double precision default null' , _tbl);
   END IF;

   EXECUTE format('ALTER TABLE qwat_od.%I
           ADD COLUMN label_1_text varchar(120)    
         , ADD COLUMN label_2_visible  smallint default 1', _tbl);

   IF _position THEN
       EXECUTE format('ALTER TABLE qwat_od.%I
           ADD COLUMN label_2_x double precision default null
         , ADD COLUMN label_2_y double precision default null', _tbl);
   END IF;
   IF _rotation THEN
       EXECUTE format('ALTER TABLE qwat_od.%I ADD COLUMN label_2_rotation double precision default null', _tbl);
   END IF;

   EXECUTE format('ALTER TABLE qwat_od.%I ADD COLUMN label_2_text varchar(120)', _tbl);

   /* Creates constraints */
   EXECUTE format('ALTER TABLE qwat_od.%$1I
           ADD CONSTRAINT %$2I FOREIGN KEY (label_1_visible) REFERENCES qwat_vl.visible(vl_code_int)
         , ADD CONSTRAINT %$3I FOREIGN KEY (label_2_visible) REFERENCES qwat_vl.visible(vl_code_int);
     CREATE INDEX %$4I ON qwat_od.%$1I(label_1_visible);
     CREATE INDEX %$5I ON qwat_od.%$1I(label_2_visible)'
   , _tbl
   , _tbl || '_label_1_visible'
   , _tbl || '_label_2_visible'
   , 'fki_' || _tbl || '_label_1_visible'
   , 'fki_' || _tbl || '_label_2_visible');
END
$func$ LANGUAGE plpgsql;
  • Be sure to use unambiguous, valid parameter names (position being a reserved word was the primary error).

  • I fixed your SQL injection issues with format() (since the simple solution with regclass didn't cover your concatenated names, as commented by @pozs). Detailed explanation:

  • _tbl has to be the unqualified table name ("tbl", not "schema.tbl").

  • Don't quote the language name, it's an identifier: LANGUAGE plpgsql

  • It's cheaper to add multiple columns / sonstraints in a single ALTER TABLE statement.

  • Other assorted simplifications / optimisations.

Sign up to request clarification or add additional context in comments.

2 Comments

While I generally agree with using regclass, this case would generate syntax errors (near constraint names) with special table names, like "$t" (it would generate f.ex. CREATE INDEX fki_"$t"_label_1_visible ... -- one should use ugly selects from pg catalogs to overcome this, or use format() / quote_ident() for simplicity
@pozs: You are right. I overlooked the concatenated names. Non-standard identifiers would be a problem. format() is the way to go.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.