Re: Regarding bug: menu_generate() too sensitive to non-existing roles

[Erik Huelsmann <..hidden..>]

>> As we were discussing on IRC, I was looking into the bug
>> "menu_generate() too sensitive to non-existing roles" (
>> https://sourceforge.net/tracker/?func=detail&aid=3445575&group_id=175965&atid=875350
>> ).
>>
>> So far, I see three solutions, some of which we already discussed on IRC:
>>
>> 1. Create a wrapper function around pg_has_role() which doesn't call
>> pg_has_role() when the role doesn't exist
>> 2. Create a view which can be used in place of menu_acl -- filtered
>> for all roles which exist
>> 3. Put the filter criteria straight into the menu_generate() and
>> menu_children() functions
>> 4. Do nothing, but when we build functionality to load add-ons into
>> setup.pl, have that verify the validity of any adjustments to the menu
>> before completion
>>
>> Since you seemed to prefer (2), I've been looking into that.
>> Unfortunately, the query optimizer seems to re-order things so that
>> the pg_has_role() function gets called on all records in menu_acl,
>> only to filter the resulting set against roles in pg_roles. By then,
>> the damage has been done. It seems this isn't an option after all.
>
> I am surprised by this, can you include your view definition?

The view is quite straight forward:

 SELECT *
   FROM menu_acl
  WHERE (menu_acl.role_name::name IN ( SELECT pg_roles.rolname
           FROM pg_roles));


> If we go with a stored procedure, I'd prefer to do it without hitting
> db rows directly.  I think this could be done by:

> 1)  If role_name is ilike 'public' return true
> 2)  Check pg_has_role and return the value
> 3)  Trap exceptions, and return false

I'm fine with this approach. The PostgreSQL documentation explicitly
states trapping exceptions comes at a measurable overhead, that's why
I thought to just go through a select statement. The role names are
part of a BTREE index, meaning you'd be able to find out about the
existence of a role in a few index iterations. (Hash lookup would
probably have been even better.)

Any comments regarding my idea of doing nothing and adding it to the
set of 'database sanity' checks to be run from setup.pl?


Bye,

Erik.