Skip to content

Use canonical category id_path equality in URL rewrite category joins - #5663

Merged
addison74 merged 8 commits into
OpenMage:mainfrom
deheerhoreca:fix/category-url-rewrite-canonical-id-path-upstream
Jul 21, 2026
Merged

Use canonical category id_path equality in URL rewrite category joins#5663
addison74 merged 8 commits into
OpenMage:mainfrom
deheerhoreca:fix/category-url-rewrite-canonical-id-path-upstream

Conversation

@loekvangool

@loekvangool loekvangool commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Summary

This changes category URL rewrite predicates from broad prefix matching to canonical equality:

  • from id_path LIKE 'category/%'
  • to id_path = CONCAT('category/', category_id)

Touched files:

  • app/code/core/Mage/Catalog/Helper/Category/Url/Rewrite.php
  • app/code/core/Mage/Core/Model/Resource/Url/Rewrite/Collection.php

Why

The broad prefix predicate can fan out inner-loop probes. In measured ANALYZE FORMAT=JSON runs, the relevant probe dropped roughly from cost ~80 to ~2 (about 40x cheaper for that join probe pattern).

Correctness / Risk

  • On clean datasets, results are expected to stay identical.
  • Differences can occur if URL rewrite rows are inconsistent (e.g. category_id and id_path disagree).
  • In that state, URL rewrite data is already not trustworthy and should be repaired regardless.
  • I'd invite others to run these queries and compare the results. My 10-year old install with almost 100k SKUs and 800 active categories, many 3rd party modules and mods, daily full dynamic category rebuilds taking 30+ minutes, has byte-for-byte the same results after this query (just much faster).

Slow:

SELECT
  `catalog_category_flat_store_1`.`entity_id`,
  `catalog_category_flat_store_1`.`name`,
  `catalog_category_flat_store_1`.`path`,
  `catalog_category_flat_store_1`.`is_active`,
  `catalog_category_flat_store_1`.`is_anchor`,
  `core_url_rewrite`.`request_path`
FROM
  `catalog_category_flat_store_1` AS `catalog_category_flat_store_1`
LEFT JOIN `core_url_rewrite`
ON
  `core_url_rewrite`.`category_id` = `catalog_category_flat_store_1`.`entity_id` AND `core_url_rewrite`.`is_system` = 1 AND `core_url_rewrite`.`store_id` = 1 AND `core_url_rewrite`.`category_id` IS NOT NULL AND `core_url_rewrite`.`id_path` LIKE 'category/%'
WHERE
  (`catalog_category_flat_store_1`.`include_in_menu` = '1') AND(`catalog_category_flat_store_1`.`is_active` = '1') AND(`catalog_category_flat_store_1`.`path` LIKE '1/2/%')
ORDER BY
  `catalog_category_flat_store_1`.`position` ASC;

Optimized:

SELECT
  `catalog_category_flat_store_1`.`entity_id`,
  `catalog_category_flat_store_1`.`name`,
  `catalog_category_flat_store_1`.`path`,
  `catalog_category_flat_store_1`.`is_active`,
  `catalog_category_flat_store_1`.`is_anchor`,
  `core_url_rewrite`.`request_path`
FROM
  `catalog_category_flat_store_1` AS `catalog_category_flat_store_1`
LEFT JOIN `core_url_rewrite`
ON
  `core_url_rewrite`.`category_id` = `catalog_category_flat_store_1`.`entity_id` AND `core_url_rewrite`.`is_system` = 1 AND `core_url_rewrite`.`store_id` = 1 AND `core_url_rewrite`.`category_id` IS NOT NULL AND `core_url_rewrite`.`id_path` = CONCAT('category/', `catalog_category_flat_store_1`.`entity_id`)
WHERE
  (`catalog_category_flat_store_1`.`include_in_menu` = '1') AND(`catalog_category_flat_store_1`.`is_active` = '1') AND(`catalog_category_flat_store_1`.`path` LIKE '1/2/%')
ORDER BY
  `catalog_category_flat_store_1`.`position` ASC;

Recovery if inconsistency is found

  • Re-save affected category/categories.
  • Reindex URL rewrites.

Closes #5662

Copilot AI review requested due to automatic review settings June 28, 2026 00:51
@github-actions github-actions Bot added Component: Core Relates to Mage_Core Component: Catalog Relates to Mage_Catalog labels Jun 28, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refines category URL rewrite predicates by replacing broad id_path LIKE 'category/%' matching with a canonical “category id_path equals category/” condition to reduce unnecessary inner-loop scans during category URL rewrite joins.

Changes:

  • Updated Mage_Catalog_Helper_Category_Url_Rewrite join predicates to use canonical category id_path equality.
  • Updated Mage_Core_Model_Resource_Url_Rewrite_Collection::filterAllByCategory() to use canonical category id_path equality.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
app/code/core/Mage/Catalog/Helper/Category/Url/Rewrite.php Replaces category id_path prefix matching with canonical equality in the helper’s URL rewrite joins.
app/code/core/Mage/Core/Model/Resource/Url/Rewrite/Collection.php Changes the “all category rewrites” filter predicate from LIKE to canonical equality.

Comment thread app/code/core/Mage/Core/Model/Resource/Url/Rewrite/Collection.php
Comment thread app/code/core/Mage/Catalog/Helper/Category/Url/Rewrite.php Outdated
Comment thread app/code/core/Mage/Catalog/Helper/Category/Url/Rewrite.php Outdated
Comment thread app/code/core/Mage/Catalog/Helper/Category/Url/Rewrite.php Outdated
Copilot AI added a commit that referenced this pull request Jul 11, 2026
…L rewrite CONCAT expressions

Apply PR #5663 changes with proper quoting - replace LIKE 'category/%' with
= CONCAT('category/', entity_id) for more precise category URL rewrite joins.

Use double-quoted strings (not escaped single quotes) to satisfy Rector's
SimplifyQuoteEscapeRector rule which was flagging the CI job failure.
@loekvangool

Copy link
Copy Markdown
Contributor Author

Adopted #5681 changes

@sonarqubecloud

Copy link
Copy Markdown

@addison74
addison74 merged commit 0efa437 into OpenMage:main Jul 21, 2026
21 checks passed
@loekvangool
loekvangool deleted the fix/category-url-rewrite-canonical-id-path-upstream branch July 21, 2026 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component: Catalog Relates to Mage_Catalog Component: Core Relates to Mage_Core

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use canonical category id_path equality instead of broad category/% predicate in URL rewrite joins

4 participants