plpgsql function issue

Question

I have the following plpgsql procedure;

DECLARE 
     _r record;
     point varchar[] := '{}';
     i int := 0;

BEGIN



FOR _r IN EXECUTE ' SELECT a.'|| quote_ident(column) || ' AS point,
       FROM ' || quote_ident (table) ||' AS a'
LOOP

       point[i] = _r;
       i = i+1;

END LOOP;

RETURN 'OK';
END;

Which its main objective is to traverse a table and store each value of the row in an array. I am still new to plpgsql. Can anyone point out is the error as it is giving me the following error;

Answer 1

@a_horse fixes most of the crippling problems with your failed attempt.

However, nobody should use this. The following step-by-step instructions should lead to a sane implementation with modern PostgreSQL.

Phase 1: Remove errors and mischief

• Remove the comma after the SELECT list to fix the syntax error.

• You start your array with 0, while the default is to start with 1. Only do this if you need to do it. Leads to unexpected results if you operate with array_upper() et al. Start with 1 instead.

• Change RETURN type to varchar[] to return the assembled array and make this demo useful.

What we have so far:

CREATE OR REPLACE FUNCTION foo(tbl varchar, col varchar)
  RETURNS varchar[] LANGUAGE plpgsql AS
$BODY$
DECLARE 
   _r record;
   points varchar[] := '{}';
   i int := 0;
BEGIN
   FOR _r IN
      EXECUTE 'SELECT a.'|| quote_ident(col) || ' AS pt
      FROM ' || quote_ident (tbl) ||' AS a'
   LOOP
      i = i + 1;        -- reversed order to make array start with 1
      points[i] = _r;
   END LOOP;

   RETURN points;
END;
$BODY$;

Phase 2: Remove cruft, make it useful

• Use text instead of character varying / varchar for simplicity. Either works, though.

• You are selecting a single column, but use a variable of type record. This way a whole record is being coerced to text, which includes surrounding parenthesis. Hardly makes any sense. Use a text variable instead. Works for any column if you explicitly cast to text (::text). Any type can be cast to text.

• There is no point in initializing the variable point. It can start as NULL here.

• Table and column aliases inside EXECUTE are of no use in this case. Dynamically executed SQL has its own scope!.

• No semicolon (;) needed after final END in a plpgsql function.

• It's simpler to just append each value to the array with || .

Almost sane:

CREATE OR REPLACE FUNCTION foo1(tbl text, col text)
  RETURNS text[] LANGUAGE plpgsql AS
$func$
DECLARE 
   point  text;
   points text[];
BEGIN
   FOR point IN
      EXECUTE 'SELECT '|| quote_ident(col) || '::text FROM ' || quote_ident(tbl)
   LOOP
      points = points || point;
   END LOOP;

   RETURN points;
END
$func$;

Phase 3: Make it shine in modern PL/pgSQL

• If you pass a table name as text, you create an ambiguous situation. You can prevent SQLi just fine with format() or quote_ident(), but this will fail with tables outside your search_path.
  Then you need to add schema-qualification, which creates an ambiguous value. 'x.y' could stand for the table name "x.y" or the schema-qualified table name "x"."y". You can't pass "x"."y" since that will be escaped into """x"".""y""". You'd need to either use an additional parameter for the schema name or one parameter of type regclass regclass is automatically quoted as need when coerced to text and is the elegant solution here.

• The new format() is simpler than multiple (or even a single) quote_ident() call.

• You did not specify any order. SELECT returns rows in arbitrary order without ORDER BY. This may seem stable, since the result is generally reproducible as long as the underlying table doesn't change. But that's 100% unreliable. You probably want to add some kind of ORDER BY.

• Finally, you don't need to loop at all. Use a plain SELECT with an Array constructor.

• Use an OUT parameter to further simplify the code

Proper solution:

CREATE OR REPLACE FUNCTION f_arr(tbl regclass, col text, OUT arr text[])
 LANGUAGE plpgsql AS
$func$
BEGIN

EXECUTE format('SELECT ARRAY(SELECT %I::text FROM %s ORDER BY 1)', col, tbl)
INTO arr;

END
$func$;

Call:

SELECT f_arr('myschema.mytbl', 'mycol');

Answer 2

This is the complete syntax (note that I renamed the parameter column to col_name as column is reserved word. The same goes for table)

create or replace function foo(col_name text, table_name text)
  returns text
as
$body$

DECLARE 
     _r record;
     point character varying[] := '{}';
     i int := 0;

BEGIN
    FOR _r IN EXECUTE 'SELECT a.'|| quote_ident(col_name) || ' AS pt, FROM ' || quote_ident (table_name) ||' AS a'
    loop
     point[i] = _r;
     i = i+1;
   END LOOP;

   RETURN 'OK';
END;
$body$
language plpgsql;

Although to be honest: I fail so see what you are trying to achieve here.