CMSPlugin.is_last_in_placeholder

[Schwankenson]

The function

def is_last_in_placeholder(self):
    """
    WARNING: this is a rather expensive call compared to is_first_in_placeholder!
    """
    return self.placeholder.cmsplugin_set.all().order_by('-position')[0].pk == self.pk

from cms.models.CMSPlugin returns true, if a plugin is the last Placeholder in a plugin. But it counts all Plugins in the placeholder, even the inline plugins.

I dont think, this is what somebody who calls the function expects (I didnt).

Maybe it would be a good idea, to pass a countinline=false parameter to the function...

[twoolie]

A Great example of the perils of leaning on the ORM too much.

Firstly, this function will fill it's cache with all attributes of every plugin in that placeholder, then it will check just the pk of the first object. I really hope this function isn't used by anyone.

A better solution would be

return self.position == self.placeholder.cmsplugin_set.all().aggregate(Max('position'))['max_position']

That way, the database can calculate the result on it's side (better queryplan), reduce data transfered over network, and get the answer quicker.

[ojii]

fixed in e6402af

[twoolie]

@czpython Your fix does nothing to address the terrible, terrible O(n^2) performance of this function.

Please consider changing the line to read either

# this will be faster if position field has an index
return self.pk == self.placeholder.cmsplugin_set.filter(parent__isnull=True).order_by('-position').only('id')[:1][0].pk

OR

# this will be faster if position field does not have an index
return self.position == self.placeholder.cmsplugin_set.filter(parent__isnull=True).aggregate(Max('position'))['max_position']

[czpython]

I don't understand how this is O(n^2), it's a single database query using indexes. Please profile it and show the results.

[ojii]

As @czpython said, this isn't O(n^2). Also the method clearly states in it's docstring that it's an expensive operation and further this is an undocumented API. As such, I'm more than happy with the patch that fixed the bug.

If you want to propose a more efficient way of solving this, please open a new ticket.

[twoolie]

@czpython You are right, it is not on it's own O( n^2 ) but using it can be.

Consider an example of a page with a placeholder which has 10 plugins inside of it.
Each of these renders using the following template.

<div class="cms-plugin cms-plugin-exampleplugin {% if plugin.is_last_in_placeholder %}last{% endif %}" >
  ...... Content .....

( I know that it's a completely synthetic worst case but where else would you use a function like this?)

Then for each of those 10 plugins on the page, you are calling this function which retrieves 10 rows from the database and throws away everything but 1 single value.

That makes 100 rows retrieved or ~O( n^2 ) performance hit just for trying to annotate whether we're last in the placeholder.

At least by attaching .only('id')[:1][0].pk or even better [:1].values('id')[0]['id'] to the end of the current solution your bringing it down to O(n) for the page rendering operation.

( I also know that I'm being anal retentive over nothing but I'm currently slogging through someone else's code that uses the simplest possible ORM calls and it's driving me insane how slow it makes everything when these functions with small performance penalties are then called in a loop. Death by a thousand cuts. And everything in cms is just so well optimised everywhere else. I'll just shut up about it now, I promise. )