Polishing.

Refine HQL rendering for duration expressions.

Retain whitespace for error message reconstruction, refine error handling.

See #3757
This commit is contained in:
Mark Paluch
2025-01-30 14:16:27 +01:00
parent 75904a91a3
commit cd12bd5693
8 changed files with 122 additions and 6 deletions

View File

@@ -837,7 +837,7 @@ reserved_word
*/
WS : [ \t\r\n] -> skip ;
WS : [ \t\r\n] -> channel(HIDDEN) ;
// Build up case-insentive tokens

View File

@@ -1560,7 +1560,7 @@ identifier
*/
WS : [ \t\r\n] -> skip ;
WS : [ \t\r\n] -> channel(HIDDEN);
// Build up case-insentive tokens

View File

@@ -817,7 +817,7 @@ reserved_word
*/
WS : [ \t\r\n] -> skip ;
WS : [ \t\r\n] -> channel(HIDDEN) ;
// Build up case-insentive tokens

View File

@@ -15,10 +15,17 @@
*/
package org.springframework.data.jpa.repository.query;
import java.util.List;
import org.antlr.v4.runtime.BaseErrorListener;
import org.antlr.v4.runtime.CommonToken;
import org.antlr.v4.runtime.InputMismatchException;
import org.antlr.v4.runtime.NoViableAltException;
import org.antlr.v4.runtime.RecognitionException;
import org.antlr.v4.runtime.Recognizer;
import org.springframework.util.ObjectUtils;
/**
* A {@link BaseErrorListener} that will throw a {@link BadJpqlGrammarException} if the query is invalid.
*
@@ -43,7 +50,48 @@ class BadJpqlGrammarErrorListener extends BaseErrorListener {
@Override
public void syntaxError(Recognizer<?, ?> recognizer, Object offendingSymbol, int line, int charPositionInLine,
String msg, RecognitionException e) {
throw new BadJpqlGrammarException("Line " + line + ":" + charPositionInLine + " " + msg, grammar, query, null);
throw new BadJpqlGrammarException(formatMessage(offendingSymbol, line, charPositionInLine, msg, e, query), grammar,
query, null);
}
/**
* Rewrite the error message.
*/
private static String formatMessage(Object offendingSymbol, int line, int charPositionInLine, String message,
RecognitionException e, String query) {
String errorText = "At " + line + ":" + charPositionInLine;
if (offendingSymbol instanceof CommonToken ct) {
String token = ct.getText();
if (!ObjectUtils.isEmpty(token)) {
errorText += " and token '" + token + "'";
}
}
errorText += ", ";
if (e instanceof NoViableAltException) {
errorText += message.substring(0, message.indexOf('\''));
if (query.isEmpty()) {
errorText += "'*' (empty query string)";
} else {
List<String> list = query.lines().toList();
String lineText = list.get(line - 1);
String text = lineText.substring(0, charPositionInLine) + "*" + lineText.substring(charPositionInLine);
errorText += "'" + text + "'";
}
} else if (e instanceof InputMismatchException) {
errorText += message.substring(0, message.length() - 1).replace(" expecting {",
", expecting one of the following tokens: ");
} else {
errorText += message;
}
return errorText;
}
}

View File

@@ -1752,7 +1752,7 @@ class HqlQueryRenderer extends HqlBaseVisitor<QueryTokenStream> {
QueryRendererBuilder builder = QueryRenderer.builder();
builder.append(visit(ctx.expression()));
builder.appendExpression(visit(ctx.expression()));
builder.append(QueryTokens.expression(ctx.BY()));
builder.appendExpression(visit(ctx.datetimeField()));

View File

@@ -27,6 +27,7 @@ import org.antlr.v4.runtime.CommonTokenStream;
import org.antlr.v4.runtime.Lexer;
import org.antlr.v4.runtime.Parser;
import org.antlr.v4.runtime.ParserRuleContext;
import org.antlr.v4.runtime.RecognitionException;
import org.antlr.v4.runtime.TokenStream;
import org.antlr.v4.runtime.atn.PredictionMode;
import org.antlr.v4.runtime.misc.ParseCancellationException;
@@ -82,7 +83,14 @@ class JpaQueryEnhancer<Q extends QueryInformation> implements QueryEnhancer {
P parser = getParser(query, lexerFactoryFunction, parserFactoryFunction);
parser.getInterpreter().setPredictionMode(PredictionMode.SLL);
parser.setErrorHandler(new BailErrorStrategy());
parser.setErrorHandler(new BailErrorStrategy() {
@Override
public void reportError(Parser recognizer, RecognitionException e) {
// avoid BadJpqlGrammarException creation in the first pass.
// recover(…) is going to handle cancellation.
}
});
try {

View File

@@ -0,0 +1,59 @@
/*
* Copyright 2025 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.data.jpa.repository.query;
import static org.assertj.core.api.Assertions.*;
import org.junit.jupiter.api.Test;
/**
* Unit tests for {@link BadJpqlGrammarException}.
*
* @author Mark Paluch
*/
class BadJpqlGrammarExceptionUnitTests {
@Test // GH-3757
void shouldContainOriginalText() {
assertThatExceptionOfType(BadJpqlGrammarException.class)
.isThrownBy(() -> JpaQueryEnhancer.HqlQueryParser
.parseQuery("SELECT e FROM Employee e WHERE FOO(x).bar RESPECTING NULLS"))
.withMessageContaining("no viable alternative")
.withMessageContaining("SELECT e FROM Employee e WHERE FOO(x).bar *RESPECTING NULLS")
.withMessageContaining("Bad HQL grammar [SELECT e FROM Employee e WHERE FOO(x).bar RESPECTING NULLS]");
}
@Test // GH-3757
void shouldReportExtraneousInput() {
assertThatExceptionOfType(BadJpqlGrammarException.class)
.isThrownBy(() -> JpaQueryEnhancer.HqlQueryParser.parseQuery("select * from User group by name"))
.withMessageContaining("extraneous input '*'")
.withMessageContaining("Bad HQL grammar [select * from User group by name]");
}
@Test // GH-3757
void shouldReportMismatchedInput() {
assertThatExceptionOfType(BadJpqlGrammarException.class)
.isThrownBy(() -> JpaQueryEnhancer.HqlQueryParser.parseQuery("SELECT AVG(m.price) AS m.avg FROM Magazine m"))
.withMessageContaining("mismatched input '.'").withMessageContaining("expecting one of the following tokens:")
.withMessageContaining("EXCEPT")
.withMessageContaining("Bad HQL grammar [SELECT AVG(m.price) AS m.avg FROM Magazine m]");
}
}

View File

@@ -1891,6 +1891,7 @@ class HqlQueryRendererTests {
assertQuery("SELECT a FROM foo a WHERE (cast(a.createdAt as date) - CURRENT_DATE()) BY day - 2 = 0");
assertQuery("SELECT a FROM foo a WHERE (cast(a.createdAt as date)) BY day - 2 = 0");
assertQuery("SELECT f.start BY DAY - 2 FROM foo f");
assertQuery("SELECT f.start - 1 minute FROM foo f");
assertQuery("SELECT f FROM foo f WHERE (cast(f.start as date) - CURRENT_DATE()) BY day - 2 = 0");