views integration, left join instead of inner join

guardian - April 26, 2008 - 21:40
Project:Radioactivity
Version:5.x-1.1
Component:Code
Category:feature request
Priority:normal
Assigned:Unassigned
Status:closed
Description

my suggestion is to change the type of join for the module from inner to left otherwise, nodes which don't have associated radioactivity information (yet) will never appear in the view

please note that the patch also contains the fix for http://drupal.org/node/251651

AttachmentSize
radioactivity.patch1 KB

#1

skiminki - April 28, 2008 - 07:32

Is this required by a real use case? I ask this because inner joins should be faster than left outer joins.

In medium/large sites you can expect 10:1, 100:1 or even greater ratio of nodes per rows in radioactivity (per decay profile) which justifies the inner join. This is the case in a site where this module was originally written for. If you really need left join, I see two possibilities:

  • I wonder if the choice of left/inner join per view can be made user-configurable in the views UI?
  • Have separate views criteria for inner and left joins, like 'Radioactivity: Node energy (@e), include zeros' and 'Radioactivity: Node energy (@e), don't include zeros (faster)'

We could also make this configurable in the radioactivity admin menu, but I wouldn't want to make this kind of setting global.

#2

guardian - April 28, 2008 - 12:21

my use case is the following: i have a view that sorts the nodes by radioactivity then by stickiness then by created time

for my home page, i extract the top node of the view and render it highlighted in a separate div so it makes sense to always have a node returned by the view

this enables me to give more importance to the latest sticky node of the site in case of low activity, but as well keep track of the currently most viewed node in case your site has been digged

if you think that performance might be a problem for medium / large sites, i would prefer having an option on the criterium like "All" and "Only Radioactive". having this option means we find a way to change the type of the join dynamically when the view is built. in case we can't do that, having 2 criteria per profile is an acceptable fallback

thx for the follow up ! i'm eager to see this polished :)

#3

skiminki - April 29, 2008 - 13:50
Status:needs review» needs work

Ok, I looked briefly into this, and attached is a patch that provides 'all' and 'radioactive only' versions of field/sort/filter criteria. However, I'm not fully comfortable with this for two reasons:

  1. I'd rather have the join type selectable in views ui. I couldn't figure out how to do that. Maybe someone more familiar with views could help?
  2. The ALL option doesn't currently work with PostgreSQL, because NULLs have infinity value in sorting. In SQL, this can be remedied using 'ORDER BY COALESCE(energy, 0)' but again, I couldn't get this working properly. BTW, I'm not sure at all that ORDER BY COALESCE(...) is able to do good job performance-wise... but I might be wrong.

I'd like to have at least the second item sorted out before inclusion. So, until someone's up to the task...

AttachmentSize
radioactivity-module-251656-3.patch 2.95 KB

#4

guardian - April 29, 2008 - 22:52

please review this one, in the end it seems to work like intended

disclaimer: i have nearly no experience in databases in general but i googled some information about the problem you mentioned regarding PostgreSQL, also this is the first time i'm coding a sort criterion for views

AttachmentSize
radioactivity-joins.patch 5.19 KB

#5

guardian - April 29, 2008 - 22:56
Status:needs work» needs review

#6

guardian - April 29, 2008 - 23:02

works as intended on mysql, since i don't have postgresql

#7

skiminki - May 2, 2008 - 11:17

Ok, I used the patch in comment 4 as a basis and came up with the attached patch. There's some notable changes (in addition to white space changes):

  • Field, filter and sort definitions has been split to use different join aliases. This means the join type defined in sort does not interfere with join types in field and filter.
  • Field uses left outer join, as energy can be considered as just another node property when displaying a node. So if it's not there the node is still displayed with null energy.
  • Filter definitions use inner join for speed reasons. This could be made configurable like the join type of sort definitions.
  • Field click sorting uses COALESCE for PostgreSQL compatibility.

Please, test this patch against the current CVS HEAD ( http://cvs.drupal.org/viewvc.py/drupal/contributions/modules/radioactivi... )

Note that existing views with radioactive energy will get broken after applying this patch.

AttachmentSize
radioactivity-module-251656-7.patch 5.44 KB

#8

guardian - May 5, 2008 - 15:17

works for me

you mentioned "field click sorting uses COALESCE for PostgreSQL compatibility" - i'm not experienced in PostgreSQL at all but the reason why my previous patch uses

<?php
$query
->orderby[] = "$table.$field IS NOT NULL $order, $table.$field $order";
?>
and not COALESCE is that google told me it was a way not to have NULLs at the top of the results when sorting

maybe your implementation of

<?php
function radioactivity_field_sort_handler_energy($fielddata, $fieldinfo) {
  return
'COALESCE('.$fielddata['tablename'].'.energy, 0)';
}
?>
could as well use IS NOT NULL instead of COALESCE depending on the performance ? you decide, as i said i don't know PostgreSQL and can't benchmark it right now

apart from the point mentioned above, ready to be committed ?

#9

skiminki - May 5, 2008 - 18:45

Ok, the radioactivity-module-251656-7.patch was almost ready for commit. Two small adjustments:

  • I removed the COALESCE from the mysql case. With PostgreSQL we just need to rely on its optimizer for now. AFAICT, we would need to write a real handler to be able to use the IS NOT NULL trick. Anyway, field click sorting and PostgreSQL combination is not probably that important, even if there was a performance penalty.
  • Renamed energy_f to energy in 'fields' section. Allows non-hardcoded sort handler.

With these two, here's the commit: http://drupal.org/cvs?commit=114748

Thanks!

#10

skiminki - May 5, 2008 - 18:45
Status:needs review» fixed

#11

Anonymous (not verified) - May 19, 2008 - 18:51
Status:fixed» closed

Automatically closed -- issue fixed for two weeks with no activity.

 
 

Drupal is a registered trademark of Dries Buytaert.