Skip to content

Commit

Permalink
Polishing.
Browse files Browse the repository at this point in the history
Refine HQL rendering for duration expressions.

Retain whitespace for error message reconstruction, refine error handling.

See #3757
  • Loading branch information
mp911de committed Jan 30, 2025
1 parent 6038bba commit 7c9b682
Show file tree
Hide file tree
Showing 8 changed files with 122 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -837,7 +837,7 @@ reserved_word
*/


WS : [ \t\r\n] -> skip ;
WS : [ \t\r\n] -> channel(HIDDEN) ;

// Build up case-insentive tokens

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1560,7 +1560,7 @@ identifier
*/


WS : [ \t\r\n] -> skip ;
WS : [ \t\r\n] -> channel(HIDDEN);

// Build up case-insentive tokens

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -817,7 +817,7 @@ reserved_word
*/


WS : [ \t\r\n] -> skip ;
WS : [ \t\r\n] -> channel(HIDDEN) ;

// Build up case-insentive tokens

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand All @@ -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;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -1752,7 +1752,7 @@ public QueryTokenStream visitFromDurationExpression(HqlParser.FromDurationExpres

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()));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
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;
Expand Down Expand Up @@ -79,7 +80,14 @@ static <P extends Parser> ParserRuleContext parse(String query, Function<CharStr
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 {

Expand Down
Original file line number Diff line number Diff line change
@@ -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]");
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -1891,6 +1891,7 @@ void arithmeticDate() {
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");
Expand Down

0 comments on commit 7c9b682

Please sign in to comment.