The cache objects returned by cache_get_multiple() have a cid property, while the cache object returned by cache_get() does not. That's quite a WTF.

I'm not sure why in DrupalDatabaseCache::getMultiple the cid column is retrieved. If it is because of SQL standards regarding the IN operator, then one solution would be to just unset this property afterwards, although removal could perhaps be considered an API change. See also #333171: Optimize the field cache.

A different approach would be to fix this in cache_get() at the cost of a little extra resource usage.

CommentFileSizeAuthor
#17 978144-get-getmultiple-inconsistencies.patch1.15 KBwebchick
#1 978144-get-getmultiple-inconsistencies.patch1.15 KBdamien tournoud

Comments

damien tournoud’s picture

damien tournoud commented
Status: Active » Needs review
StatusFileSize
new 978144-get-getmultiple-inconsistencies.patch1.15 KB

I see no reason not to simply do this (it's not like this is a cheap function and that the additional call could possibly matter).

Status: Needs review » Needs work

The last submitted patch, 978144-get-getmultiple-inconsistencies.patch, failed testing.

eric_a’s picture

eric_a commented
Status: Needs work » Needs review

#1: 978144-get-getmultiple-inconsistencies.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 978144-get-getmultiple-inconsistencies.patch, failed testing.

eric_a’s picture

eric_a commented
Status: Needs work » Needs review

#1: 978144-get-getmultiple-inconsistencies.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 978144-get-getmultiple-inconsistencies.patch, failed testing.

eric_a’s picture

eric_a commented
Status: Needs work » Needs review

#1: 978144-get-getmultiple-inconsistencies.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 978144-get-getmultiple-inconsistencies.patch, failed testing.

eric_a’s picture

eric_a commented
Status: Needs work » Needs review

#1: 978144-get-getmultiple-inconsistencies.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 978144-get-getmultiple-inconsistencies.patch, failed testing.

eric_a’s picture

eric_a commented

After applying this patch locally on my standard profile install I got
Fatal error: Class 'SelectQuery_mysql' not found in C:\xampplite\htdocs\drupal\includes\database\database.inc on line 685

eric_a’s picture

eric_a commented

I created #983132: DrupalDatabaseCache::getMultiple assumes it can rely on class autoloading.

eric_a’s picture

eric_a commented
Status: Needs work » Needs review

I just found #851136: Make the database autoloading more robust. It just got committed.

eric_a’s picture

eric_a commented

#1: 978144-get-getmultiple-inconsistencies.patch queued for re-testing.

eric_a’s picture

eric_a commented

#1: 978144-get-getmultiple-inconsistencies.patch queued for re-testing.

eric_a’s picture

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

This "approach" needs to be taken sooner or later.

#851136: Make the database autoloading more robust was rolled back and then committed again with parts of it extended for pgsql and sqllite. I pushed the button again, waiting for the green.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
StatusFileSize
new 978144-get-getmultiple-inconsistencies.patch1.15 KB

Testbot appears to be stuck.

dries’s picture

dries commented

I don't see a reason not to simplify the code like this.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Me neither. Just wanted to make sure testbot was cool with it, since it was frozen for awhile yesterday.

Committed to HEAD. Thanks!

Status: Fixed » Closed (fixed)

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