-
-
Notifications
You must be signed in to change notification settings - Fork 6.9k
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
Add support for index / query hints in query builder #19946
base: master
Are you sure you want to change the base?
Conversation
uaoleg
commented
Sep 2, 2023
•
edited
Loading
edited
Q | A |
---|---|
Is bugfix? | ❌ |
New feature? | ✔️ |
Breaks BC? | ❌ |
Fixed issues | #9119 |
PR Summary
|
Note that this syntax is planned to be deprecated in MySQL:
https://dev.mysql.com/doc/refman/8.0/en/index-hints.html And new syntax can already be handled by |
@uaoleg Looks good. Please also add unit tests and a line in the changelog.md |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is definitely not finished:
- There are no tests that would verify that generated queries actually work.
- Implementation uses MySQL specific syntax, so it would fail for the rest of DBMS.
- This MySQL syntax is planned to be removed, so the only part of this PR that works, is already soft-deprecated.
IMO instead of providing API for FORCE INDEX
clause, we should just provide API for adding something after table name (including table name for JOIN) - it should be easier to maintain and more flexible for users (even index hints for MySQL are more complicated than just FORCE INDEX
). If you need index hints, you're probably targeting very specific DBMS with specific version, so having abstraction at framework level does not bring much benefit.