Skip to content

Conversation

mart-insiders
Copy link

Q A
Bug fix? yes
New feature? yes
BC breaks? yes
Deprecations? yes
Fixed tickets comma separated list of tickets fixed by the PR

@dannyvw dannyvw requested a review from Copilot August 20, 2025 08:17
Copy link

@Copilot Copilot AI left a comment

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 refactors the redirect query mechanism for performance improvements by adding new columns and optimizing the database query strategy. The changes move away from using wildcard pattern matching in SQL to a more efficient approach using pre-computed pattern and prefix columns.

  • Adds origin_pattern and origin_prefix columns to the Redirect entity for optimized querying
  • Refactors the repository query logic to use the new columns with more efficient SQL operations
  • Includes database migration instructions for updating existing data

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

File Description
src/Kunstmaan/RedirectBundle/Repository/RedirectRepository.php Refactored query logic to use new origin_pattern and origin_prefix columns with optimized SQL
src/Kunstmaan/RedirectBundle/Entity/Redirect.php Added origin_pattern and origin_prefix properties with lifecycle callbacks and database indexes
UPGRADE-7.X.md Added migration instructions for the new columns and data updates

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

UPGRADE-7.X.md Outdated
- Add a line to the migration to update all redirect entities:
```php
$this->addSql("UPDATE kuma_redirects SET origin_pattern = REPLACE(origin, '*', '%');");
$this->addSql("ALTER TABLE kuma_redirects ADD COLUMN origin_prefix VARCHAR(255) GENERATED ALWAYS AS (SUBSTRING_INDEX(origin_pattern, '/', 2)) STORED;");
Copy link

Copilot AI Aug 20, 2025

Choose a reason for hiding this comment

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

The SUBSTRING_INDEX function extracts only the first two parts separated by '/', but the repository code expects the full prefix up to a certain length. This mismatch between the generated column logic and the query logic will cause incorrect results.

Suggested change
$this->addSql("ALTER TABLE kuma_redirects ADD COLUMN origin_prefix VARCHAR(255) GENERATED ALWAYS AS (SUBSTRING_INDEX(origin_pattern, '/', 2)) STORED;");
$this->addSql("ALTER TABLE kuma_redirects ADD COLUMN origin_prefix VARCHAR(255) GENERATED ALWAYS AS (LEFT(origin_pattern, 255)) STORED;");

Copilot uses AI. Check for mistakes.

namespace Kunstmaan\RedirectBundle\Repository;

use Doctrine\DBAL\Connection;
use Doctrine\DBAL\Query\QueryBuilder;
Copy link

Copilot AI Aug 20, 2025

Choose a reason for hiding this comment

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

The QueryBuilder import is not used in the visible code. Consider removing unused imports to keep the code clean.

Suggested change
use Doctrine\DBAL\Query\QueryBuilder;

Copilot uses AI. Check for mistakes.

@mart-insiders mart-insiders merged commit 32d8849 into insiders Aug 26, 2025
10 of 20 checks passed
@mart-insiders mart-insiders deleted the KUN-10126 branch August 26, 2025 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants