Skip to content

Add optimizer hint query api#18688

Merged
markstory merged 1 commit into
5.nextfrom
optimizer-hints
May 31, 2025
Merged

Add optimizer hint query api#18688
markstory merged 1 commit into
5.nextfrom
optimizer-hints

Conversation

@othercorey
Copy link
Copy Markdown
Contributor

@othercorey othercorey commented May 26, 2025

refs #18140

Example mysql index hint:
https://dev.mysql.com/doc/refman/8.4/en/index-hints.html

$query->index('USE', ['my_index1', 'my_index2']);
$query->index('IGNORE', 'my_index3');
select * from table USE INDEX (my_index1, my_index2) IGNORE INDEX (my_index3)

Example mysql optimizer hint (added in 8.4 with plans to warning to deprecate index hint but not done so):
https://dev.mysql.com/doc/refman/8.4/en/optimizer-hints.html

$query->hint('INDEX', 'my_index');
select /*+ INDEX(my_index) */ from table

The postgresql extension has syntax compatible with mysql hint syntax so the api should work for both:
https://github.com/ossc-db/pg_hint_plan/blob/master/docs/hint_details.md

@othercorey othercorey added this to the 5.3.0 milestone May 26, 2025
Comment thread src/Database/Query/SelectQuery.php Outdated
@ADmad
Copy link
Copy Markdown
Member

ADmad commented May 26, 2025

For the index() method how would one specify the FOR part?

@othercorey
Copy link
Copy Markdown
Contributor Author

If we don't mind supporting only mysql 8.4+, we don't need the legacy index hint api and can just use the optimizer hint.

Comment thread src/Database/Query/SelectQuery.php Outdated
Comment thread src/Database/Query/SelectQuery.php Outdated
@ADmad
Copy link
Copy Markdown
Member

ADmad commented May 27, 2025

Optimizer hints can be used for various query types, not just SELECT statements. So are we limiting them to just SelectQuery as we don't foresee usage for other queries?

@othercorey
Copy link
Copy Markdown
Contributor Author

othercorey commented May 27, 2025

Optimizer hints can be used for various query types, not just SELECT statements. So are we limiting them to just SelectQuery as we don't foresee usage for other queries?

No, we can make optimizer hints generic. I used SelectQuery for the RFC because the original index hints were limited. I'm not really sure the syntax for index hints in update statements but I assume it's right after the table name.

@ADmad
Copy link
Copy Markdown
Member

ADmad commented May 27, 2025

I'm not really sure the syntax for index hints in update statements but I assume it's right after the table name.

From the mysql docs:

The parser recognizes optimizer hint comments after the initial keyword of SELECT, UPDATE, INSERT, REPLACE, and DELETE statements.

@othercorey
Copy link
Copy Markdown
Contributor Author

For the optimizer hint, but not for the legacy index hint.

@ADmad
Copy link
Copy Markdown
Member

ADmad commented May 27, 2025

For the optimizer hint, but not for the legacy index hint.

Right, I was only asking about the hint() method for optimizer hints. Given that legacy index hints are being phased out having them only for SelectQuery (if at all) should be okay I guess.

@ADmad
Copy link
Copy Markdown
Member

ADmad commented May 27, 2025

Seems SQL Server also has query hints which are specified using the OPTION clause at the end of the query

https://learn.microsoft.com/en-us/sql/t-sql/queries/select-transact-sql?view=sql-server-ver16
https://learn.microsoft.com/en-us/sql/t-sql/queries/hints-transact-sql-query?view=sql-server-ver16#syntax

@ADmad
Copy link
Copy Markdown
Member

ADmad commented May 27, 2025

Oracle uses the special comment format similar to MySQL and pg_hint_plan.

https://docs.oracle.com/cd/B10500_01/server.920/a96533/hintsref.htm

@othercorey
Copy link
Copy Markdown
Contributor Author

Seems SQL Server also has query hints which are specified using the OPTION clause at the end of the query

https://learn.microsoft.com/en-us/sql/t-sql/queries/select-transact-sql?view=sql-server-ver16 https://learn.microsoft.com/en-us/sql/t-sql/queries/hints-transact-sql-query?view=sql-server-ver16#syntax

This syntax varies. I think it simplest way to support it would be to put the entire value into $name.

@othercorey
Copy link
Copy Markdown
Contributor Author

For the optimizer hint, but not for the legacy index hint.

Right, I was only asking about the hint() method for optimizer hints. Given that legacy index hints are being phased out having them only for SelectQuery (if at all) should be okay I guess.

Do we think it's worth supporting both? I don't mind, but it could get confusing since index hint is mysql only. Probably not that confusing though.

@ADmad
Copy link
Copy Markdown
Member

ADmad commented May 28, 2025

I think it simplest way to support it would be to put the entire value into $name.

I am wondering if instead of having $name and $params we should just have a $hint argument, using which you can specify the full hint string as needed. That would simply the implementation for all dbs. This feature is not going to be commonly used.

Do we think it's worth supporting both? I don't mind, but it could get confusing since index hint is mysql only. Probably not that confusing though.

I am wondering the same, since index hints are mysql only and planned to be phased out. It's been over a year since mysql 8.4 is out and wouldn't it be safe to assume that those who need this feature would be able to install the required mysql version?

@othercorey othercorey changed the title Add optimzer and index hint query api Add optimzer query api May 28, 2025
@othercorey othercorey force-pushed the optimizer-hints branch 3 times, most recently from 67e2276 to e7c0d7d Compare May 28, 2025 09:34
@othercorey othercorey changed the title Add optimzer query api Add optimizer hint query api May 28, 2025
Comment thread src/Database/Driver/Mysql.php
Comment thread src/Database/QueryCompiler.php Outdated
@othercorey othercorey force-pushed the optimizer-hints branch 3 times, most recently from a4d85f5 to 36906d5 Compare May 30, 2025 09:04
@othercorey othercorey marked this pull request as ready for review May 30, 2025 09:04
@othercorey othercorey requested a review from ADmad May 30, 2025 09:05
Comment thread src/Database/QueryCompiler.php Outdated
/**
* @var string Optimizer hint comment opening
*/
protected const HINT_COMMENT_OPEN = '/*+';
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are these constants necessary? We have templates for other clauses (though I see we can't just add a string for this to $_templates due to the way it's used).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Looks like Oracle uses the same prefix so I'm going to hard-code it for now instead.

@othercorey
Copy link
Copy Markdown
Contributor Author

Renamed query api and clause to optimizerHint.

@markstory markstory merged commit b3fa0b3 into 5.next May 31, 2025
12 of 13 checks passed
@markstory markstory deleted the optimizer-hints branch May 31, 2025 16:15
@othercorey othercorey requested a review from Copilot June 1, 2025 17:06
Copy link
Copy Markdown
Contributor

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

Adds support for passing engine-specific optimizer hints via a new optimizerHint() API, and integrates hint comments into compiled SQL when the driver supports them.

  • Introduces optimizerHint clause in all query types (Select, Insert, Update, Delete)
  • Extends QueryCompiler to render hint comments based on a new OPTIMIZER_HINT_COMMENT feature flag
  • Adds comprehensive tests for hint behavior and adjusts driver support maps

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/TestCase/Database/QueryTest.php Added testOptimizerHintClause() for the new clause
tests/TestCase/Database/QueryCompilerTest.php Tests for SQL compilation of optimizer hints
src/Database/QueryCompiler.php Hooked _buildOptimizerHintPart() into SQL builders
src/Database/Query/SelectQuery.php Added optimizerHint to select query parts
src/Database/Query/InsertQuery.php Added optimizerHint to insert query parts
src/Database/Query/UpdateQuery.php Added optimizerHint to update query parts
src/Database/Query/DeleteQuery.php Added optimizerHint to delete query parts
src/Database/Query.php Added optimizerHint() API to base Query class
src/Database/DriverFeatureEnum.php New OPTIMIZER_HINT_COMMENT feature enum
src/Database/Driver/Sqlserver.php Marked hints unsupported
src/Database/Driver/Sqlite.php Marked hints unsupported
src/Database/Driver/Postgres.php Marked hints supported
src/Database/Driver/Mysql.php Marked hints supported
Comments suppressed due to low confidence (2)

tests/TestCase/Database/QueryCompilerTest.php:225

  • [nitpick] The method name has typos and is inconsistent: rename to testUpdateWithOptimizerHint to match other test naming conventions.
public function testUpdateWitOptimizerhHint(): void

tests/TestCase/Database/QueryCompilerTest.php:209

  • [nitpick] The test name omits 'Hint'—consider renaming to testUpdateWithOptimizerHint for consistency with the other hint tests.
public function testUpdateWithOptimizer(): void

/**
* Builds the optimizer hint comment part.
*
* @param list<string> $parts The optmizer hints
Copy link

Copilot AI Jun 1, 2025

Choose a reason for hiding this comment

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

Typo in docblock: change 'optmizer' to 'optimizer'.

Suggested change
* @param list<string> $parts The optmizer hints
* @param list<string> $parts The optimizer hints

Copilot uses AI. Check for mistakes.
arshidkv12 pushed a commit to arshidkv12/cakephp that referenced this pull request Sep 6, 2025
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.

4 participants