Posted by earnie on June 29, 2009 at 12:50pm
| Project: | Advanced User |
| Version: | 6.x-2.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | critical |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
Any time you filter by role you get an unexpected list.
Comments
#1
I can confirm this. It seems to work when the role EQ is selected but not when the role NE is selected. Great idea and much needed module. Hope you get it working.
Phil
#2
Subscribe, have exactly the same problem. Sorry, on D5
I understand all of this is a lot of work, but why not start selling subscriptions to updates of your module. You would make money and alot of people would not be so frustrated anymore. E-commerce or Ubercart allow it all to be done automatically. Why is this market not functioning?
Theo Richel
I am trying to select the users who have not donated to my site. The role option doesnt work, but others (payment smaller than some date, payment smaller than some amount) are dysfunctional as well.
So I gave the users I do *not* want to select a special permission and selected for users who do NOT CONTAIN or are NOPT EQUIVALENT that permission. Then I get an error message:
user warning: You have an error in your SQL syntax. Check the manual that corresponds to your MySQL server version for the right syntax to use near 'SELECT ur.uid FROM GRK_users_roles ur WHERE ur.rid NOT IN (SELE query: SELECT COUNT(DISTINCT u.uid) FROM GRK_users u LEFT JOIN GRK_users_roles ur ON u.uid = ur.uid LEFT JOIN GRK_profile_values profile_vrnm ON profile_vrnm.fid = 2 AND profile_vrnm.uid = u.uid LEFT JOIN GRK_profile_values profile_achtrnm ON profile_achtrnm.fid = 3 AND profile_achtrnm.uid = u.uid LEFT JOIN GRK_profile_values profile_pstadr ON profile_pstadr.fid = 4 AND profile_pstadr.uid = u.uid LEFT JOIN GRK_profile_values profile_pstco ON profile_pstco.fid = 5 AND profile_pstco.uid = u.uid LEFT JOIN GRK_profile_values profile_plts ON profile_plts.fid = 6 AND profile_plts.uid = u.uid LEFT JOIN GRK_profile_values profile_cntry ON profile_cntry.fid = 7 AND profile_cntry.uid = u.uid LEFT JOIN GRK_profile_values profile_datum1 ON profile_datum1.fid = 18 AND profile_datum1.uid = u.uid LEFT JOIN GRK_profile_values profile_bedrag1 ON profile_bedrag1.fid = 19 AND profile_bedrag1.uid = u.uid LEFT JOIN GRK_profile_values profile_reknr ON profile_reknr.fid = 30 AND profile_reknr.uid = u.uid LEFT JOIN GRK_profile_values profile_boekje ON profile_boekje.fid = 23 AND profile_boekje.uid = u.uid LEFT JOIN GRK_profile_values profile_bio ON profile_bio.fid = 16 AND profile_bio.uid = u.uid WHERE u.uid != 0 AND ( ((u.uid NOT IN (SELECT ur.uid FROM GRK_users_roles ur WHERE ur.rid NOT IN (SELECT p.rid FROM GRK_permission p WHERE p.perm != 'make backups'))) AND u.uid != 1)) in /home/www/klimatosoof/includes/database.mysql.inc on line 174.
If I do the same selection but then with EQ instead of NE to look after a permission I get the same error message.
I have the impression that when you once select to search in user roles it stays in that modus.
#3
This is not easy to fix properly but the following works for drupal 5 version. Should also work same for 6 but not tested.
replace line (around 422) in advuser_filters.inc:
$_where = $op.' '.str_ireplace("%op", $qop, $filters[$key]['where']);with the following:
// hack to fix negative searches in rolesif ($key == 'user_roles' && $qop == '!=' ) {
$_where = "u.uid not in (select uid from {users_roles} where rid = %d)";
}
else {
$_where = $op.' '.str_ireplace("%op", $qop, $filters[$key]['where']);
}
The problem is that you can't formulate an SQL query using the != operator which produces the right results (only users which don't have a particular role). Ive got around it by using a sub query for the one special case.
#4
Other than saying a meaningless I'm sorry, no news. I've had zero time to look at these. I'll check it out in January 2010.
#5
The attached patch fixes some of the filter issues with roles. Since the roles can have multiple values per user, certain combination of filter operations are next to impossible to implement only through SQL queries.
Previously, it was not possible to combine two role filters (BTW, this is an issue with the regular user module too). With this patch, that should be possible:
e-g (Role is ) AND (Role is ) should now work.
#6
Your need to watch your tab characters. You need to convert tabs to spaces. Also the use of a JOIN clause would be a better use of resources than an IN clause.
#7
I will change the default tab->space settings in my eclipse ide for php. This is more of a product of years of practice/convention followed.
I agree that JOINs would be better than IN, but in this case, I am not sure how it could be done. The way filter UI to query building is done, I am not sure if a JOIN could be added. I will look at this further and see if that is possible. The implementation is very similar to what is done to filter by permission.
Thanks for the review.
#8
The query builder does support constructing JOIN fragments and so, It might be possible to do this using JOINs alone; while we are at it, it might be prudent to take out the LEFT JOIN that the base query does with user_roles. This is not used anywhere (except in the role filter, which is broken anyway).
So, taking the extra base LEFT JOIN might actually improve the performance little bit!
#9
I've got a bit of time to give to fixing this bug, but I'm not able to replicate it.
I'm using the most recent 6.x-2.x version checked out from CVS - CHANGELOG.txt looks like this:
$Id: CHANGELOG.txt,v 1.1.2.3 2009/06/23 15:33:25 earnie Exp $
Advanced User 6.x-2, 2009-06-23
-----------------------------------
#470604: Need to remember the page the action was executed from.
Advanced User 6.x-2, 2009-05-07
-----------------------------------
#377282: Clicking UPDATE OPTIONS does not work.
#427170: "This version is incompatible with the 6.10 version of Drupal core".
Clean Drupal 6.15 install, create 50 users with Devel generate, allocate roles to each of these (some users with multiple roles). Any role-based filtering I apply seems to work.
Could someone please give me the steps that allow for the replication of this bug on a clean (-ish) Drupal 6 install?
#10
Sorry only got drupal 5 here with this module.
However, can you confirm that if you filter on "role not equal to X" that
1) none of the users that pass through that filter have role X (even where they have other roles).
2) no users are filtered out that shouldn't be (on the grounds that they have some other role which is not equal to X).
#11
You're totally correct. I had misread the UI and thought that the box where you select EQ, NE etc only applied to the Permission filter, as that's where it's located on the screen.
I've renamed the issue so it's a bit clearer. I'm going to start adding a few UI issues as well.
I'll get working on this later. Thanks.
#12
Don't forget there is also the issue raised by LKM (see #5 onwards) which doesn't fit the new title. Possibly a single fix could address both issues.
#13
A useful way to debug this is to build the same query with Views 2.
In Views 2, a "give me all the users without a certain role" query looks like this:
SELECT users.uid AS uid,users.name AS users_name
FROM users users
LEFT JOIN users_roles users_roles ON users.uid = users_roles.uid AND users_roles.rid = 12
WHERE users_roles.rid IS NULL
Whereas in Advuser, the same query will look like this ($sql defined at line 106 of the module):
SELECT DISTINCT u.uid, u.name, u.status, u.created, u.access FROM {users} u LEFT JOIN {users_roles} ur ON u.uid = ur.uid WHERE u.uid != 0 AND ( ur.rid != %d) ORDER BY u.created DESCCleaned up to look like the Views SQL:
SELECT users.uid AS uid,users.name AS users_name
FROM users users
LEFT JOIN users_roles users_roles ON users.uid = users_roles.uid
WHERE users.uid !=0 AND (users_roles.rid != 12)
Running these queries through on PHPMyAdmin or Navicat or whatever shows that the Advuser query gives more results than the Views query, and digging around in the users_roles table quickly shows why: if a user has more than one role, then they will have multiple entries in users_roles: one entry for each role.
I'm not entirely sure why the Views SQL works, but it does and if it's good enough for merlinofchaos then it's good enough for me.
Not sure how to factor this in to the existing Advuser code, but this at least nails down the problem.
#14
Seems like you are on the right track. Good luck with this- its tricky and Im afraid I can't give it time at the moment. I'm still using the ugly hack I posted in #3 which is keeping my users happy for the time being.
#15
Thanks mitchh. I'm going to work through the query builder and see if I can understand how it works.
One thing that does strike me, though: why don't we make Views a dependency of the module and then delegate the query building to Views?
I'm even thinking that it would be possible to produce a clone of Advanced User using Views and Views Bulk Operations. Is there a reason this hasn't been done?
I feel a little bad about this as I've volunteered to help out on the module and it now looks like I'm trying to destroy it, but I don't want to have feature duplication if we can easily avoid it.
#16
I suspect it might be simply because the module was built before views became as powerful as they are now. Earnie?
Still, not something to be undertaken lightly I would have thought.
#17
@mitchh: I've proposed that version 3.x is based on Views and VBO - see #713562: Allow use Views and Views Bulk Operations if they are enabled..
In the meantime I'll continue to work on critical 2.x bugs like this one. I've been through the code and made it a little easier to read; I'll try and get a quick fix together for this bug.
#18
As #713562 has been suspended- is anyone looking at how to fix this now?
#19
I'll be debugging it in the next few days. I'll take a look at the patch at #5 to see if I can glean anything from it.
#20
The problem is converting the operators to appropriate set operations:
#3 is about translating "role != roleA" to "roleA not in roles"
I think #5 is about being able to do roleA in roles AND roleB in roles when cascading
Some operators clearly are not relevant.
#21
The patch attached at #765776-8: 6.x-3.x changes semi-resolves this issue. Provided is a selection for 'No roles assigned' which you can use in combination with OR is equal to SOMEROLE. You still can't select only users in multiple roles but for now it is a won't fix.
#22
True story: A site for a charity has a 'member' role for subscribing members. Recently the membership secretary used this module to try and email all non-members to request the joining fee. As well as emails going to all the users who were non-members, emails were also sent to the treasurer, the editors, various affiliates and (luckily) the membership secretary himself. All these members got a confusing email because they also had a role which was not equal to 'member' and so passed the filter. This is what prompted the fix in #3.
Although this may not technically be a bug, I think the membership secretary's expectations of how it would work were reasonable. I think people should be warned about falling into this hole if the issue is not going to be fixed.
#23
The redesign gives a specific warning when the Role field is selected. Please have a look at it and open a new issue if you think more wording is required.
I too think that what is trying to be done is reasonable. The NOT IN (list of uid) isn't feasible due to limitations of memory and the sizes of some users databases. So my option remains to warn the user about the limitations.
#24
Thanks, that is probably the best that can be done until someone figures out a clever join or something to do the job properly.
Mitch
#25
What about a NOT IN (sub-SELECT)?
#26
See #23- earnie reckons this may cause the database some memory problems.
If the sub select contained 100K UIDs represented as integers then this would take up 400K in memory.
I have to say I'm not convinced this would necessarily cause problems.
#27
No, NOT IN (list of uids) would potentially be a very very long SQL statement. This is definitely a bad idea.
NOT IN (sub-SELECT) would be handled inside the database engine — at the limit it could be somewhat slow, but the database engine will do whatever is necessary to perform the query within the given constraints.
#28
Automatically closed -- issue fixed for 2 weeks with no activity.