hubzero / hubzero-cms

Platform for Scientific Collaboration
https://hubzero.org
GNU General Public License v2.0
47 stars 57 forks source link

Remove unused left join from jos_resources display query #1698

Closed jsperhac closed 6 months ago

jsperhac commented 6 months ago

There's no ticket or Jira card associated with this. It's just an obvious error I saw that is easily fixed.

The following query is found in the 'display.php' script for com_resources views:

$query = "SELECT DISTINCT R.id, R.title, R.type, R.logical_type AS logicaltype, AA.subtable, R.created, R.created_by, R.published, R.publish_up, R.standalone, R.rating, R.times_rated, R.alias, R.ranking, rt.type AS typetitle "; $query .= "FROM #__author_assoc AS AA, #__resource_types AS rt, #__resources AS R "; $query .= "LEFT JOIN #__resource_types AS t ON R.logical_type=t.id "; $query .= "WHERE AA.authorid = ". User::get('id') ." "; $query .= "AND R.id = AA.subid "; $query .= "AND AA.subtable = 'resources' "; $query .= "AND R.standalone=1 AND R.type=rt.id AND (R.published=2 OR R.published=3) AND R.type!=7 "; $query .= "ORDER BY published ASC, title ASC";

On inspection, the LEFT JOIN to jos_resource_types joins on the wrong column. The resulting table alias, t, is entirely unused in the query; the incorrect left join ensures that there's no effect on the result set.

Elsewhere in the query, jos_resource_types is specified with alias rt and is correctly joined to jos_resources R in the WHERE clause, on R.type = rt.id

So this is a one-line correction, and removes an unused table alias with an incorrect join condition. The proposed correction looks like this (only the LEFT JOIN is removed):

$query = "SELECT DISTINCT R.id, R.title, R.type, R.logical_type AS logicaltype, AA.subtable, R.created, R.created_by, R.published, R.publish_up, R.standalone, R.rating, R.times_rated, R.alias, R.ranking, rt.type AS typetitle "; $query .= "FROM #__author_assoc AS AA, #__resource_types AS rt, #__resources AS R "; $query .= "WHERE AA.authorid = ". User::get('id') ." "; $query .= "AND R.id = AA.subid "; $query .= "AND AA.subtable = 'resources' "; $query .= "AND R.standalone=1 AND R.type=rt.id AND (R.published=2 OR R.published=3) AND R.type!=7 "; $query .= "ORDER BY published ASC, title ASC";

jsperhac commented 6 months ago

TODO in the future, when this code is touched: evaluate

zooley commented 6 months ago

the LEFT JOIN to jos_resource_types joins on the wrong column

It is the correct column but it's not being used. The query joins on the #__resource_types table twice: one (inner) JOIN for type and one LEFT JOIN for logical_type as the logical_type is optional but type is required. The issue is that the SELECT isn't doing anything with that join (e.g., t.type AS logicaltitle). Most spots in the code where it'd potentially grab the logical type title, it's pulling the entire logical type record via the model's relationship ($row->logicaltype->get('type')), BUT logicaltitle is referenced in a couple spots. This JOIN shows up in a few queries, such as here and here.

So, you'll probably need to either add t.type AS logicaltitle to the SELECT, ensure the query being modified here isn't one that generates those references later on, or check/modify all references to not rely on that SELECT value (the aforementioned model relationship).

jsperhac commented 6 months ago

Thanks for the clarification on this @zooley, that's much appreciated. I'll close this PR, figuring that it can serve as a sort of documentation of logical_type.

Looking at the 7373 published resources on Nanohub, I saw 842 with these logical_type titles associated; the most recent examples of such resources date to 2008.

Example query: published non-tool resources with logical types:

SELECT DISTINCT R.id, R.title,  
    rt.type as type_title,  
    t.type as logicaltype_title, 
    R.created, u.name as created_by, 
    R.publish_up 
FROM jos_resource_types AS rt, jos_resources AS R 
LEFT JOIN jos_resource_types AS t ON R.logical_type=t.id 
LEFT JOIN jos_users as u on R.created_by = u.id
WHERE 
    R.standalone=1 AND R.type!=7 
    AND R.type=rt.id 
    AND R.published = 1 
    AND t.type is not null
    ORDER BY published DESC, title ASC

Result set includes e.g. logical_type_example_resources