Select and use one column

Please consider the following (working) stored procedure. This function receives an integer as its first parameter, indicating which privilegeid has to be checked against the current user. Privileges are stored in the table privileges and consist of an id and a name (varchar).

Each privilege belongs to one or more roles stored in users_roles. Each user is assigned to one or more roles. This function retrieves all roles assigned to current_user and checks them, like said, against the given privilige开发者_运维百科id.

CREATE OR REPLACE FUNCTION "public"."has_privilege" (in int4) RETURNS bool AS
$BODY$
DECLARE
    asked_privilegeid ALIAS FOR $1;
    userid int;
    role_row users_roles%rowtype;
    privilege_row privileges%rowtype;
BEGIN
    EXECUTE 'SELECT userid FROM users WHERE username=$1' INTO userid USING current_user;

    FOR role_row IN SELECT * FROM users_roles
    WHERE userid=userid 
    LOOP
    IF role_row.roleid = 1 THEN
        return TRUE;
    END IF;

    FOR privilege_row IN SELECT * FROM privileges WHERE roleid=role_row.roleid LOOP
        IF privilege_row.privilegeid = asked_privilegeid THEN
            return TRUE;
        END IF;
    END LOOP;
    END LOOP;

    return FALSE;
END
$BODY$
LANGUAGE 'plpgsql'

However, this code isn't a efficient as it could be considering it retrieves all rowvalues for users_roles and privileges. I tried to write the procedure as following, but it doesn't seem to work:

CREATE OR REPLACE FUNCTION "public"."has_privilege" (in int4) RETURNS bool AS
$BODY$
DECLARE
    asked_privilegeid ALIAS FOR $1;
    privilegeid int;
    userid int;
    roleid int;

    //role_row users_roles%rowtype;
    //privilege_row privileges%rowtype;
BEGIN
    EXECUTE 'SELECT userid FROM users WHERE username=$1' INTO userid USING current_user;

    FOR roleid IN SELECT roleid FROM users_roles
    WHERE userid=userid 
    LOOP
    IF roleid = 1 THEN
        return TRUE;
    END IF;

    FOR privilegeid IN SELECT privilegeid FROM privileges WHERE roleid=roleid LOOP
        IF privilegeid = asked_privilegeid THEN
            return TRUE;
        END IF;
    END LOOP;
    END LOOP;

    return FALSE;
END
$BODY$
LANGUAGE 'plpgsql'

What am I doing wrong? Thanks in advance!

Edit: The indentions didn't came through as expected. Here are pastebin links: http://pastebin.com/w18WaCW0 http://pastebin.com/W8ewXxEe

The problem is with the row

WHERE userid=userid 

It is ambiguous which is the column from the table and which is your variable. Same issue here

FROM privileges WHERE roleid=roleid 

Don't use variable names that are also column names that you will reference

You can also rewrite your PROC body as a direct SQL statement that will probably work faster

CREATE OR REPLACE FUNCTION "public"."has_privilege" (in int4) RETURNS bool AS
$BODY$
DECLARE
    asked_privilegeid ALIAS FOR $1;
BEGIN

RETURN EXISTS (
    SELECT *
    FROM users u
    INNER JOIN users_roles r on r.userid=u.userid
    LEFT JOIN privileges p on p.roleid=r.roleid
       AND p.privilegeid = asked_privilegeid
       AND r.roleid <> 1 //  don't need to process this join if we already have our answer
    WHERE u.username = $1
      AND (r.roleid=1 OR p.privilegeid is not null))

END
$BODY$
LANGUAGE 'plpgsql'

DECLARE
    asked_privilegeid ALIAS FOR $1;
    _userid int; -- chage to avoid variable name same as column name
    role_row users_roles%rowtype;
    privilege_row privileges%rowtype;
BEGIN
    _userid := (select userid from users where username = session_user); -- if u use current_user u will have problem when function is defined as security definer

    FOR role_row IN SELECT * FROM users_roles WHERE userid = _userid -- your code is error becus userid is same as your variable name
    LOOP
    IF role_row.roleid = 1 THEN
        return TRUE;
    END IF;

    FOR privilege_row IN SELECT * FROM privileges WHERE roleid=role_row.roleid LOOP
        IF privilege_row.privilegeid = asked_privilegeid THEN
            return TRUE;
        END IF;
    END LOOP;
    END LOOP;

    return FALSE;
END

when u declare variable in postgre, it is good practice that u use underscore b4 variable name '_userid'. to make it distinct from column name

I think this can be solved with just a single SELECT statement:

SELECT count(*)
FROM privileges p
  JOIN roles r ON r.privilegeid = p.privilegeid 
  JOIN user_roles ur ON ur.roleid = r.roleid
  JOIN users u ON u.userid = ur.userid AND u.username = session_user
WHERE p.privilegeid = asked_privilegeid

(not tested)

If the count is zero, the privilege is not assigned, otherwise it is.