Skip to content
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

HHH-19215 Extends Dialect#addQueryHints to support straight_join syntax #9825

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

hogeunchung
Copy link

@hogeunchung hogeunchung commented Mar 2, 2025

The goal is to expand the coverage of the query pattern as both MySQL and SingleStore support straight_join syntax, and their dialect classes rely on Dialect class to add index hints.

Example use case:

Session session = entityManager.unwrap(Session.class);
String query = "select * from table1 straight_join table2 on table1.column1 = table2.column1 where table1.column2 = 'value1' and table1.column3 = 'value2'";

List result = session
       .createNativeQuery(query, Table1.class)
       .addQueryHint("index_column3")
       .getResultList();

While there may be cases where this is useful, it could be considered a relatively unique situation.
If this change conflicts with the original intent, I understand if it is not accepted.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license
and can be relicensed under the terms of the LGPL v2.1 license in the future at the maintainers' discretion.
For more information on licensing, please check here.


https://hibernate.atlassian.net/browse/HHH-19215

QUERY_PATTERN regex handles queries with straight_join
@@ -283,7 +283,7 @@ public abstract class Dialect implements ConversionContext, TypeContributor, Fun
private static final Pattern ESCAPE_CLOSING_COMMENT_PATTERN = Pattern.compile( "\\*/" );
private static final Pattern ESCAPE_OPENING_COMMENT_PATTERN = Pattern.compile( "/\\*" );
private static final Pattern QUERY_PATTERN = Pattern.compile(
"^\\s*(select\\b.+?\\bfrom\\b.+?)(\\b(?:natural )?(?:left |right |full )?(?:inner |outer |cross )?join.+?\\b)?(\\bwhere\\b.+?)$");
"^\\s*(select\\b.+?\\bfrom\\b.+?)(\\b(?:(?:natural )?(?:left |right |full )?(?:inner |outer |cross )?join |straight_join).+?\\b)?(\\bwhere\\b.+?)$");
Copy link
Member

Choose a reason for hiding this comment

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

It looks like the pattern might benefit from being more explicit about necessary whitespace:

Suggested change
"^\\s*(select\\b.+?\\bfrom\\b.+?)(\\b(?:(?:natural )?(?:left |right |full )?(?:inner |outer |cross )?join |straight_join).+?\\b)?(\\bwhere\\b.+?)$");
"^\\s*(select\\s.+?\\sfrom\\s.+?)(\\s(?:(?:natural\\s+)?(?:left\\s+|right\\s+|full\\s+)?(?:inner\\s|outer\\s|cross\\s)?join\\s|straight_join\\s).+?\\b)?(\\swhere\\s.+?)$");

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for the suggestion.
You're right; the original regex can't handle some queries with new lines.

Based on your suggestion, I've modified the regex as follows:

  1. \\b -> \\s for improved robustness
  2. (?:A\\s|B\\s|C\\s)? -> (?:A|B|C)?\\s* for better readability

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.

2 participants