The code in SqlContentEntityStorage::countFieldData() tries to optimise for the hasData use case but does not quite manage to do that.

CommentFileSizeAuthor
#2 2388765.2.patch742 bytesalexpott
#2 0-2-interdiff.txt598 bytesalexpott
drupal8.countFieldData.patch733 bytesalexpott

Comments

Status: Needs review » Needs work

The last submitted patch, drupal8.countFieldData.patch, failed testing.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Needs work » Needs review
StatusFileSize
new 0-2-interdiff.txt598 bytes
new 2388765.2.patch742 bytes

Fixed it. And yay we have good test coverage.

amateescu’s picture

amateescu commented
Status: Needs review » Reviewed & tested by the community

Nice catch!

chx’s picture

chx commented

I wonder whether #2345063: Mutating Select::countQuery() might produce invalid SQL or mask a performance bug would've have solved this had that issue not devolve into another fight :(

  • catch committed d3e81f5 on 8.0.x 
    Issue #2388765 by alexpott: Improve performance of...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

@chx it'll likely fix similar queries, which is why I agree that issue is major.

However in this case we just do not want the full table count at all, so the patch here would need to be done regardless of that issue.

Committed/pushed to 8.0.x, thanks!

Status: Fixed » Closed (fixed)

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