Skip to content

HHH 19695 fix broken rendering of FETCH { FIRST | NEXT } clause #10724

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

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 38 additions & 3 deletions hibernate-core/src/main/java/org/hibernate/sql/Template.java
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ public static String renderWhereStringTemplate(
boolean inExtractOrTrim = false;
boolean inCast = false;
boolean afterCastAs = false;
boolean inFetchClause = false; // Track if we're in FETCH FIRST/NEXT clause

boolean hasMore = tokens.hasMoreTokens();
String nextToken = hasMore ? tokens.nextToken() : null;
Expand Down Expand Up @@ -218,6 +219,16 @@ else if ( quotedIdentifier && dialect.closeQuote()==token.charAt(0) ) {
final boolean quotedOrWhitespace =
quoted || quotedIdentifier || isQuoteCharacter
|| token.isBlank();

// Check for FETCH FIRST/NEXT clause
if ( !quotedOrWhitespace && !inFetchClause && isFetchClauseStart(sql, symbols, tokens, lcToken)) {
inFetchClause = true;
}
// Reset FETCH clause state when we encounter certain tokens
else if ( !quotedOrWhitespace && inFetchClause && isFetchClauseEnd(token, lcToken)) {
inFetchClause = false;
}

if ( quotedOrWhitespace ) {
result.append( token );
}
Expand Down Expand Up @@ -249,8 +260,9 @@ else if ( inCast && ("as".equals( lcToken ) || afterCastAs) ) {
}
else if ( !inFromClause // don't want to append alias to tokens inside the FROM clause
&& isIdentifier( token )
&& !isFunctionOrKeyword( lcToken, nextToken, dialect, typeConfiguration )
&& !isLiteral( lcToken, nextToken, sql, symbols, tokens ) ) {
&& !isFunctionOrKeyword( lcToken, nextToken, dialect, typeConfiguration, inFetchClause )
&& !isLiteral( lcToken, nextToken, sql, symbols, tokens )
&& !inFetchClause ) { // Don't prefix tokens that are part of FETCH clause
result.append(alias)
.append('.')
.append( dialect.quote(token) );
Expand Down Expand Up @@ -286,6 +298,20 @@ else if ( inFromClause && ",".equals(lcToken) ) {
return result.toString();
}

private static boolean isFetchClauseStart(String sql, String symbols, StringTokenizer tokens, String lcToken) {
return "fetch".equals( lcToken ) && lookPastBlankTokens( sql, symbols, tokens, 1,
nextNonBlank -> {
if ( nextNonBlank == null ) return false;
final String n = nextNonBlank.toLowerCase( Locale.ROOT );
return "first".equals( n ) || "next".equals( n );
}
);
}

private static boolean isFetchClauseEnd(String token, String lcToken) {
return "only".equals( lcToken ) || ( !isNumeric( token ) && !Set.of("rows", "row", "first", "next").contains(lcToken) );
}

private static boolean endsWithDot(String token) {
return token != null && token.endsWith( "." );
}
Expand Down Expand Up @@ -389,7 +415,8 @@ private static boolean isFunctionOrKeyword(
String lcToken,
String nextToken,
Dialect dialect,
TypeConfiguration typeConfiguration) {
TypeConfiguration typeConfiguration,
boolean inFetchGrammar) {
if ( "(".equals( nextToken ) ) {
return true;
}
Expand All @@ -398,6 +425,10 @@ else if ( SOFT_KEYWORDS.contains( lcToken ) ) {
// TODO: treat 'current date' as a function
return false;
}
// Treat 'first' and 'next' as keywords ONLY within FETCH clause
else if ( "first".equals( lcToken ) || "next".equals( lcToken ) ) {
return inFetchGrammar;
}
else {
return KEYWORDS.contains( lcToken )
|| isType( lcToken, typeConfiguration )
Expand All @@ -423,4 +454,8 @@ private static boolean isBoolean(String token) {
default -> false;
};
}

private static boolean isNumeric(String token) {
return token.matches( "\\d+" );
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -89,4 +89,5 @@ public void testIntegerOverflowForMaxResults() {
sql.contains("fetch next ? rows only")
);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,61 @@ public void templateLiterals(SessionFactoryScope scope) {
"CAST({@}.foo AS signed)", factory );
}

@Test
@JiraKey("HHH-19695")
public void testFetchGrammarVsColumnNames(SessionFactoryScope scope) {
SessionFactoryImplementor factory = scope.getSessionFactory();

// Test that "first" and "next" are treated as keywords when part of FETCH grammar
assertWhereStringTemplate( "fetch first 10 rows only", "fetch first 10 rows only", factory );
assertWhereStringTemplate( "fetch next 5 rows only", "fetch next 5 rows only", factory );
assertWhereStringTemplate( "select * from table fetch first 1 row only",
"select * from table fetch first 1 row only", factory );

// Mixed scenarios: ensure identifiers around FETCH grammar are still qualified
assertWhereStringTemplate( "select first_name from users fetch first 10 rows only",
"select {@}.first_name from users fetch first 10 rows only", factory );
assertWhereStringTemplate( "where fetch_count > 5 and fetch next 1 row only",
"where {@}.fetch_count > 5 and fetch next 1 row only", factory );
assertWhereStringTemplate( "select first from users fetch first 10 rows only",
"select {@}.first from users fetch first 10 rows only", factory );
assertWhereStringTemplate( "select next from users fetch next 10 rows only",
"select {@}.next from users fetch next 10 rows only", factory );
}

@Test
@JiraKey("HHH-19695")
public void testFetchGrammarVariants(SessionFactoryScope scope) {
SessionFactoryImplementor factory = scope.getSessionFactory();
Dialect dialect = factory.getJdbcServices().getDialect();

// Variants of FETCH FIRST/NEXT
assertWhereStringTemplate( "fetch first 1 row only", "fetch first 1 row only", factory );
assertWhereStringTemplate( "fetch next 10 rows only", "fetch next 10 rows only", factory );

// Parameterized row count
assertWhereStringTemplate( "fetch next ? rows only", "fetch next ? rows only", factory );

// Casing variants
assertWhereStringTemplate( "FETCH First 10 ROWS ONLY", "FETCH First 10 ROWS ONLY", factory );

// Extra whitespace and newlines
assertWhereStringTemplate( "fetch first 10 rows only", "fetch first 10 rows only", factory );
assertWhereStringTemplate( "fetch\nfirst 3 rows only", "fetch\nfirst 3 rows only", factory );

// State reset after ONLY: trailing 'next' should be qualified
assertWhereStringTemplate( "fetch next 1 rows only and next > 5",
"fetch next 1 rows only and {@}.next > 5", factory );

// Qualified identifier should remain as-is
assertWhereStringTemplate( "select u.first from users u fetch first 1 row only",
"select u.first from users u fetch first 1 row only", factory );

// Quoted identifier should be qualified, while FETCH clause remains unqualified
assertWhereStringTemplate( "select `first` from users fetch first 1 row only",
"select {@}." + dialect.quote("`first`") + " from users fetch first 1 row only", factory );
}

private static void assertWhereStringTemplate(String sql, SessionFactoryImplementor sf) {
assertEquals( sql,
Template.renderWhereStringTemplate(
Expand Down