current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 5 down vote favorite
1

I am creating a user permission system. I will have my user permissions set up off of "objects" and not pages. For example, I have an EVENTS section. Within that section, there are FORMS, FEES, etc. I want to be able to handle permissions down to the child levels.

Here is my permission_objects table:

| id | object | parent_id |
+----+--------+-----------+
| 1  | EVENTS | NULL      |
| 2  | FORMS  | 1         |
| 3  | FEES   | 1         |

Here is my current query:

SELECT `up`.*
FROM `permission_objects` `po`
INNER JOIN `user_permissions` `up` ON `up`.`object`=`po`.`id` AND (`up`.`user_group`=2 OR `up`.`user_id`=17)
WHERE `po`.`object`='EVENTS' OR `po`.`parent_id`=(
  SELECT `id` FROM `permission_objects` WHERE `object`='EVENTS'
)

I really want to avoid doing a subquery. I thought that instead of having a parent_id I could have parent_object and have that contain the parent's object value. i.e. | 2 | FORMS | EVENTS |. That way I could change my WHERE clause to:

WHERE `po`.`object`='EVENTS' OR `po`.`parent_object`='EVENTS'

However I feel that keeping it as parent_id is a better approach. Is there a way to keep that but not have to have a subquery in my sql?

share|improve this question

2 Answers 2

up vote 3 down vote accepted

The sub-query as a concept is not a problem... you need to do that work, and this is a decent way to do it.

It would be better if it was expressed as a direct lookup... like:

WHERE `po`.`object`='EVENTS'
   OR exists (
      SELECT `sub`.`id`
      FROM `permission_objects` `sub`
      WHERE `sub`.`object`='EVENTS'
        and `sub`.`id` = `po`.`parent_id`
)

This sub-select is better because it can do an indexed lookup in the table in the sub-select based on the id column.

Apart from that, I don't see a better, or neater way to do it.

share|improve this answer
    
Did you mean and sub.id = po.parent_id? –  Aust Mar 21 '14 at 17:40
    
@Aust Yes, I did ;-) –  rolfl Mar 21 '14 at 17:46
    
I thought so but I've never seen exists so I wasn't exactly sure. =) I do have one question though, how does exists compare to adding LEFT JOIN permission_objects po2 ON po2.parent_id=po.id and changing ... up ON (up.object=po.id OR up.object=po2.id) ... –  Aust Mar 21 '14 at 18:00
    
The left-join wont give you what you want because you need the OR condition on the parent's Event..... and that will produce duplicate results. exists is the right tool for this job. You do have an index on the permission_objects.id column, right? –  rolfl Mar 21 '14 at 18:05
    
Yes I have my id columns indexed. –  Aust Mar 21 '14 at 18:49
up vote 2 down vote

Your query probably doesn't do what you intend, because AND has higher precedence that OR. It is therefore equivalent to

SELECT `up`.*
    FROM `permission_objects` `po`
        INNER JOIN `user_permissions` `up`
            ON (`up`.`object`=`po`.`id` AND `up`.`user_group`=2)
            OR `up`.`user_id`=17
    WHERE `po`.`object`='EVENTS'
        OR `po`.`parent_id`=(
            SELECT `id` FROM `permission_objects` WHERE `object`='EVENTS'
        )

You probably meant

SELECT `up`.*
    FROM `permission_objects` `po`
        INNER JOIN `user_permissions` `up`
            ON `up`.`object`=`po`.`id`
            AND (`up`.`user_group`=2 OR `up`.`user_id`=17)
    WHERE `po`.`object`='EVENTS'
        OR `po`.`parent_id`=(
            SELECT `id` FROM `permission_objects` WHERE `object`='EVENTS'
        )

However, that's not really a join condition. I'd move the user_group and user_id filter to the WHERE clause. Since you are interested in the user_permissions more than the permission_objects, I'd make user_permissions the primary table.

SELECT `up`.*
    FROM `user_permissions` `up`
        INNER JOIN `permission_objects` `po`
            ON `up`.`object` = `po`.`id`
    WHERE
        (`up`.`user_group` = 2 OR `up`.`user_id` = 17)
        AND (
            `po`.`object`='EVENTS'
             OR `po`.`parent_id`=(
                 SELECT `id` FROM `permission_objects` WHERE `object`='EVENTS'
             )
         )
share|improve this answer
    
Yes you're right. I meant to put that OR in (). Oops! ... Are there performance gains from using user_permissions as the primary table instead of permission_objects? –  Aust Mar 22 '14 at 19:00
    
Using user_permissions as the primary table is a stylistic change; performance should be the same. –  200_success♦ Mar 22 '14 at 19:55

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.