Copilot commented on code in PR #16719:
URL: https://github.com/apache/grails-core/pull/16719#discussion_r4343849923
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/HqlQueryContext.java:
##########
@@ -184,6 +184,42 @@ static Class<?> getTarget(CharSequence hql, Class<?>
clazz) {
};
}
+ /** Unwraps only parentheses enclosing the whole query for analysis,
without changing the executed HQL. */
+ private static @Nullable String unwrapRootQuery(@Nullable String hql) {
+ if (hql == null) return null;
+ String s = hql.trim();
+ while (s.startsWith("(") && s.endsWith(")")) {
+ int depth = 0;
+ int closing = -1;
+ int i = 0;
+ while (i < s.length()) {
+ char c = s.charAt(i);
+ if (c == '\'' || c == '"' || c == '`') {
+ i = SqlRestriction.skipQuoted(s, i, c);
+ continue;
+ }
+ if (s.startsWith("/*", i)) {
+ int commentEnd = s.indexOf("*/", i + 2);
+ i = commentEnd < 0 ? s.length() : commentEnd + 2;
+ continue;
+ }
Review Comment:
The root-paren scanner skips only block comments (`/* ... */`) but not
end-of-line SQL/HQL comments (commonly `-- ...`). If a `--` comment contains a
`)` (or `(`), the depth tracking can become incorrect and cause
`unwrapRootQuery` to return the wrong string (either failing to unwrap when it
should, or unwrapping when it shouldn’t). Consider extending the scanner to
detect `--` at position `i` and skip to the end of the string (or newline, if
present) similarly to the block-comment handling.
##########
grails-data-hibernate7/core/src/test/groovy/grails/gorm/tests/HqlSelectResultTypeSpec.groovy:
##########
@@ -71,6 +71,29 @@ class HqlSelectResultTypeSpec extends
HibernateGormDatastoreSpec {
books*.title == ['Emma', 'Persuasion', 'The Hobbit']
books.every { it instanceof HqlResultTypeBook }
}
+
+ void 'executeQuery infers scalar results from a parenthesized root query:
#hql'() {
+ expect:
+ HqlResultTypeBook.executeQuery(hql).sort() == ['Emma', 'Persuasion',
'The Hobbit']
+
+ where:
+ hql << [
+ '(select b.title from HqlResultTypeBook b)',
+ '((select b.title from HqlResultTypeBook b))',
+ '( select b.title from HqlResultTypeBook b )',
+ "(select b.title from HqlResultTypeBook b where b.title <>
')')",
+ '(/* ignore ) and ( */ select b.title from HqlResultTypeBook
b)',
Review Comment:
There’s good coverage for block comments and string literals containing `)`,
but the new scanning logic in `unwrapRootQuery` also interacts with other
comment/quoting patterns. If Hibernate/HQL in this project supports `--` line
comments or quoted identifiers (e.g., backticks/double-quotes), consider adding
at least one test case for those patterns so regressions in the scanner don’t
silently change unwrapping behavior.
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/HqlQueryContext.java:
##########
@@ -184,6 +184,42 @@ static Class<?> getTarget(CharSequence hql, Class<?>
clazz) {
};
}
+ /** Unwraps only parentheses enclosing the whole query for analysis,
without changing the executed HQL. */
+ private static @Nullable String unwrapRootQuery(@Nullable String hql) {
+ if (hql == null) return null;
+ String s = hql.trim();
+ while (s.startsWith("(") && s.endsWith(")")) {
+ int depth = 0;
+ int closing = -1;
+ int i = 0;
+ while (i < s.length()) {
+ char c = s.charAt(i);
+ if (c == '\'' || c == '"' || c == '`') {
+ i = SqlRestriction.skipQuoted(s, i, c);
Review Comment:
`unwrapRootQuery` relies on `SqlRestriction.skipQuoted` to always advance
`i`. If `skipQuoted` ever returns the same index (e.g., on malformed/unclosed
quotes) this loop can become non-progressing. A small defensive guard (e.g., if
the returned index is `<= i`, then break/return `s`) would make this safer and
prevent accidental infinite loops in edge cases.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
