Skip to content

Comments

Enhance index hint validation for multiple indexes#58505

Merged
taylorotwell merged 2 commits intolaravel:12.xfrom
FlexIDK:patch-1
Jan 26, 2026
Merged

Enhance index hint validation for multiple indexes#58505
taylorotwell merged 2 commits intolaravel:12.xfrom
FlexIDK:patch-1

Conversation

@FlexIDK
Copy link
Contributor

@FlexIDK FlexIDK commented Jan 26, 2026

Copilot AI review requested due to automatic review settings January 26, 2026 09:25
Copy link

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

Updates MySQL index hint validation to support specifying multiple indexes in a single hint string (e.g., useIndex('user_id, PRIMARY')), addressing #58504.

Changes:

  • Splits the provided index hint string on commas and validates each index name individually.
  • Preserves existing SQL generation while allowing comma/whitespace-separated index lists.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 113 to 118
$indexes = array_map('trim', explode(',', $index));
foreach ($indexes as $idx) {
if (! preg_match('/^[a-zA-Z0-9_$]+$/', $idx)) {
throw new InvalidArgumentException('Index name contains invalid characters.');
}
}
Copy link

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

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

The new comma-separated index parsing/validation should be covered by a unit test (e.g., useIndex('user_id, PRIMARY') producing use index (user_id, PRIMARY)), since tests/Database/DatabaseQueryBuilderTest.php currently only asserts single-index behavior for MySQL. Adding a test will prevent regressions and clarify intended whitespace/comma handling.

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@shaedrich shaedrich left a comment

Choose a reason for hiding this comment

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

It would probably be better if an array could be passed


if (! preg_match('/^[a-zA-Z0-9_$]+$/', $index)) {
throw new InvalidArgumentException('Index name contains invalid characters.');
$indexes = array_map('trim', explode(',', $index));
Copy link
Contributor

Choose a reason for hiding this comment

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

You could use first-class callable notation here:

Suggested change
$indexes = array_map('trim', explode(',', $index));
$indexes = array_map(trim(...), explode(',', $index));

$indexes = array_map('trim', explode(',', $index));
foreach ($indexes as $idx) {
if (! preg_match('/^[a-zA-Z0-9_$]+$/', $idx)) {
throw new InvalidArgumentException('Index name contains invalid characters.');
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be helpful to know which index was the problem:

Suggested change
throw new InvalidArgumentException('Index name contains invalid characters.');
throw new InvalidArgumentException('Index name "' . $idx . '" contains invalid characters.');

@taylorotwell taylorotwell merged commit 38eff27 into laravel:12.x Jan 26, 2026
70 checks passed
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

Successfully merging this pull request may close these issues.

3 participants