Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PageTable fields with sortfields causes N+1 SQL queries #1971

Open
tuomassalo opened this issue Sep 14, 2024 · 3 comments
Open

PageTable fields with sortfields causes N+1 SQL queries #1971

tuomassalo opened this issue Sep 14, 2024 · 3 comments

Comments

@tuomassalo
Copy link

Short description of the issue

Bad performance: if a PageTable field has sortfields, the sorting makes an SQL query (or more) for each subpage.

Expected behavior

Fetching a PageTable field does not cause a big number of unnecessary SQL queries.

Actual behavior

For each subpage, the sort algorithm queries each sortfield from on the fly. If a PageTable has 50 subpages and three sort fields, this means 150 extra SQL queries.

Optional: Suggestion for a possible fix

PR coming up.

Steps to reproduce the issue

  1. Add a template, e.g. mytpl
  2. To mytpl, add text field mytext
  3. Install the PageTable modules
  4. Create a PageTable field ptfield
  5. On Details tab, set Sort fields to mytext
  6. Add a PageTable field ptfield to basic-page
  7. Edit a basic page: add a few subpages to ptfield. Enter something to mytext for each one.
  8. Run this code:
$page = $wire->pages->get("id=1015"); // replace with the page id
$foo = $page->ptfield;

$queries = $wire->database->getQueryLog();
echo '<p>Number queries: ' . count($queries) . '</p>';
foreach($queries as $query){
    echo '<p>' . htmlspecialchars($query) . '</p>';
}
  1. Observe that for each subpage, the output has a line like:
SELECT field_mytext.data AS `mytext__data` FROM `field_mytext` WHERE field_mytext.pages_id=1021

Setup/Environment

  • ProcessWire version: latest dev branch, vanilla installation
  • (Optional) PHP version: 8.2
  • (Optional) MySQL version: MariaDB 10.5
@ryancramerdesign
Copy link
Member

I'll sometimes aim for more simple 1-index select queries rather than a single query with multiple joins because the multiple simple queries can often consume less time than the single more complex query. Though it depends on the case, and I don't remember for this case specifically. Though the mentioned query is just the sort of type that might perform more quickly as independent queries than in a larger query, though can't say for certain without benchmarking. When it comes to sorting, performance wise it's preferable to sort on a native pages table property when possible, such as name, sort, created, modified, id, etc.

@tuomassalo
Copy link
Author

About performance and prefething:

I've done some benchmarking by fetching pages with 10 or even 50 joined fields. Even with low network latencies (docker on local dev machine), using joins seems to perform about three times faster.

$pageIds = [/* list of 100 pages */];
$fields = [/* list of 50 basic text fields */];

foreach($pageIds as $pageId){
  $page = $wire->pages->get("id=$pageId", ["loadOptions" => ["joinFields" => $fields]]);
  // vs.
  // $page = $wire->pages->get("id=$pageId");
    
  foreach($fields as $field){
    $foo = $page->get($field);
  }
}

I might be missing something in my perf tests, but, with any number of fields, I would be surprised to find a common scenario where the database was slower to perform one SELECT with joins than N+1 simpler SELECT clauses. After all, EXPLAIN looks very good for the join selects that PW generates.

@tuomassalo
Copy link
Author

One concern about my PR, though: am I correct that setting joinFields in getById() ignores any autojoin fields that the template might have, thus actually (potentially) increasing the total number of queries, if those fields are queried later?

Thus, I believe the patch should ideally merge sortfields and autojoin fields, if the template has that set.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants