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; enter image description here

• user330315: You are missing the create function part. And you need to quote the body of the function, e.g. using dollar-quoting. Please read the manual for details.
• IT_info: I am using the GUI of postgis to create the function so there is no need to do the 'create function' in the function code cause it is done automatically. @a_horse_with_no_name
• Craig Ringer: @IT_info By "The GUI of PostGIS" do you mean PgAdmin-III? That's what the screenshot suggests. If so, can you please explain step-by-step how you're attempting to create a function this way? It sure looks to me like the surrounding CREATE [OR REPLACE] FUNCTION block isn't being sent. Maybe look at the underlying query logs?
• IT_info: The problem was that I was working on sql as a language rather than plpgsql. thanks for help
• user330315: You are also missing loop and end loop to define the scope of the for loop. Also you can't use table or column names dynamically as you do.

Answer 1

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.

• IT_info: Basically what I want is to have all the points in array so that through the use of google map api I get all the elvations of the points. Is it the right way to do it or I on the wrong approach? @a_horse_with_no_name
• user330315: @IT_info: if you want to "do" something with those values, you will need more code. If you want those values as a result of the function, you need to change the return type of the function.
• IT_info: That I understand I have to make the return type to be array right? @a_horse_with_no_name. BUT the concept is right. would it be possible to read each value from the javascript side to get the elevation of each point?

Answer 2

@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');