processwire / processwire-issues

ProcessWire issue reports.
45 stars 2 forks source link

Nonexistent values are not cached with `field=myfield`, causing unneeded SQL queries #1970

Open tuomassalo opened 2 months ago

tuomassalo commented 2 months ago

Short description of the issue

If I loop pages fetched with find("template=$template, field=mytext"), calling $page->mytext makes a database query for each page that do not have mytext value. This causes an "N+1 queries problem", potentially making a lot of unneeded queries.

Expected behavior

field=mytext should cache both the value and absence of a value, and no further database queries is needed when $page->mytext is accessed.

Actual behavior

For each page without mytext, $page->mytext triggers the query

SELECT field_mytext.data AS `mytext__data` FROM `field_mytext` WHERE field_mytext.pages_id=1015`

Optional: Suggestion for a possible fix

I suspect this to be a regression of https://github.com/processwire/processwire/commit/4344df336b0e8839e71c809b37d0a73bd1d7c2f5.

I believe the code should read:

if(strpos($key, '__')) {
    if($value === null) {
        $row[$key] = 'null'; // ensure detected by later isset in foreach($joinFields)
    }
    $page->setFieldValue($key, $value, false); // <--- always do this, as before the regression
} else {
    $page->setForced($key, $value);
}

Steps to reproduce the issue

  1. Add a few pages
  2. Add a text field mytext to basic-page
  3. Fill mytext for one page
  4. Create and run this file:
<?php
$processwirePath = '/var/www/html/';
include($processwirePath . 'index.php');

$pages = $wire->pages->find("template=basic-page, field=mytext");

foreach($pages as $page) {
    echo $page->mytext;
}

$queries = $wire->database->getQueryLog();
echo '<p>Number queries: ' . count($queries) . '</p>';
foreach($queries as $query){
    echo '<p>' . htmlspecialchars($query) . '</p>';
}
  1. See that queries like the one above is executed for each page without mytext.

Setup/Environment

tuomassalo commented 1 week ago

AFAICT, this issue was fixed by https://github.com/processwire/processwire/commit/5481d713abea02bc69508f14dcead5d6afbf85a1#diff-550599e541299002e1af2d187d0e0007d419d9584436b58075ba426c3b4c17eaL701-R702

matjazpotocnik commented 5 days ago

@tuomassalo, thanks for the update! So, can this issue be closed?