phplist

NOTE:: Before reporting an issue, make sure you are running the latest version, currently 3.3.1


View Issue Details Jump to Notes ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0017609phplist applicationMessage Send Processpublic14-02-15 08:5606-07-15 18:04
Reporterduncanc 
PrioritynormalSeverityminorReproducibilityalways
StatusresolvedResolutionfixed 
PlatformOSOS Version
Product Version3.0.12 
Target Version3.0.XFixed in Version3.2.0 
Summary0017609: Use of join without an ON clause
DescriptionThere is an sql query that uses JOIN without an ON clause, which means that it is joining every row from one table with every row from the second table. The use of DISTINCT hides this problem.

See file actions/processqueue.php line 795

    $query
    = ' select distinct u.id'
    . ' from %s as listuser'
    . ' cross join %s as u'
    . ' cross join %s as listmessage'
    . ' left join %s as um'
    . ' on (um.messageid = ? and um.userid = listuser.userid)'
    . ' where true'
    . ' and listmessage.messageid = ?'
    . ' and listmessage.listid = listuser.listid'
    . ' and u.id = listuser.userid'
    . ' and um.userid IS NULL'
    . ' and u.confirmed and !u.blacklisted and !u.disabled'
    . ' %s %s';

Also, it would be clearer to use INNER JOIN instead of CROSS JOIN. Although these are equivalent in mysql, in other sql they have different meanings.


TagsNo tags attached.
Attached Files

- Relationships

-  Notes
(0055931)
support (administrator)
14-02-15 22:42


interesting. Have you done any tests to see if there's a speed difference?
(0055932)
support (administrator)
14-02-15 22:43

hang on, there is an ON in that query, isn't there?
(0055933)
duncanc (developer)
15-02-15 12:23

There are three joins, the last of which has an ON condition, the first two do not. But you have put the ON conditions in the WHERE clause of the query and it looks like mysql uses those to avoid the cartesian product that I was thinking about.

But it is a really confusing approach and likely to be error prone by not putting the conditions in the JOIN itself

I suggest moving conditions to the joins

join %s as u on listuser.userid = u.id
join %s as listmessage on listuser.listid = listmessage.listid
(0055950)
michiel (manager)
27-02-15 09:38

https://github.com/michield/phplist/commit/ba5a576c7d6e0406b6f5d69d067505e406d30be5 [^]


Copyright © 2000 - 2017 MantisBT Team
Powered by Mantis Bugtracker