Skip to content

Commit 0ef5217

Browse files
minleejaeminleejae
authored
refactor: share expression parsing fallback logic (#2844)
1 parent ce3dce3 commit 0ef5217

2 files changed

Lines changed: 152 additions & 77 deletions

File tree

‎src/main/java/net/sf/jsqlparser/parser/CCJSqlParserUtil.java‎

Lines changed: 23 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -278,50 +278,37 @@ private static ColDataType parseColDataType(String columnDataType, boolean allow
278278
}
279279
}
280280

281-
@SuppressWarnings("PMD.CyclomaticComplexity")
282281
public static Expression parseExpression(String expressionStr, boolean allowPartialParse,
283282
Consumer<CCJSqlParser> consumer) throws JSQLParserException {
284283
if (expressionStr == null || expressionStr.isEmpty()) {
285284
return null;
286285
}
287286

288-
Expression expression = null;
289-
// first, try to parse fast and simple
287+
// First require a complete simple parse, even when the caller allows a partial result.
290288
try {
291-
CCJSqlParser parser = newParser(expressionStr).withAllowComplexParsing(false);
292-
if (consumer != null) {
293-
consumer.accept(parser);
294-
}
295-
try {
296-
expression = parser.Expression();
297-
if (parser.getNextToken().kind != CCJSqlParserTokenManager.EOF) {
298-
throw new JSQLParserException(
299-
"could only parse partial expression " + expression.toString());
300-
}
301-
} catch (ParseException ex) {
302-
throw new JSQLParserException(expressionStr, ex);
303-
}
304-
} catch (JSQLParserException ex1) {
305-
// when fast simple parsing fails, try complex parsing but only if it has a chance to
306-
// succeed
307-
CCJSqlParser parser = newParser(expressionStr).withAllowComplexParsing(true);
308-
if (consumer != null) {
309-
consumer.accept(parser);
310-
}
311-
try {
312-
expression = parser.Expression();
313-
if (!allowPartialParse
314-
&& parser.getNextToken().kind != CCJSqlParserTokenManager.EOF) {
315-
throw new JSQLParserException(
316-
"could only parse partial expression " + expression.toString());
317-
}
318-
} catch (JSQLParserException ex) {
319-
throw ex;
320-
} catch (ParseException ex) {
321-
throw new JSQLParserException(ex);
289+
return parseExpression(expressionStr, false, false, consumer);
290+
} catch (JSQLParserException ex) {
291+
return parseExpression(expressionStr, true, allowPartialParse, consumer);
292+
}
293+
}
294+
295+
private static Expression parseExpression(String expressionStr, boolean allowComplexParsing,
296+
boolean allowPartialParse, Consumer<CCJSqlParser> consumer) throws JSQLParserException {
297+
CCJSqlParser parser = newParser(expressionStr).withAllowComplexParsing(allowComplexParsing);
298+
if (consumer != null) {
299+
consumer.accept(parser);
300+
}
301+
302+
try {
303+
Expression expression = parser.Expression();
304+
if (!allowPartialParse && parser.getNextToken().kind != CCJSqlParserTokenManager.EOF) {
305+
throw new JSQLParserException(
306+
"could only parse partial expression " + expression.toString());
322307
}
308+
return expression;
309+
} catch (ParseException ex) {
310+
throw new JSQLParserException(ex);
323311
}
324-
return expression;
325312
}
326313

327314
/**
@@ -356,50 +343,9 @@ public static Expression parseCondExpression(String condExpr, boolean allowParti
356343
});
357344
}
358345

359-
@SuppressWarnings("PMD.CyclomaticComplexity")
360346
public static Expression parseCondExpression(String conditionalExpressionStr,
361347
boolean allowPartialParse, Consumer<CCJSqlParser> consumer) throws JSQLParserException {
362-
if (conditionalExpressionStr == null || conditionalExpressionStr.isEmpty()) {
363-
return null;
364-
}
365-
366-
Expression expression = null;
367-
// first, try to parse fast and simple
368-
try {
369-
CCJSqlParser parser =
370-
newParser(conditionalExpressionStr).withAllowComplexParsing(false);
371-
if (consumer != null) {
372-
consumer.accept(parser);
373-
}
374-
try {
375-
expression = parser.Expression();
376-
if (parser.getNextToken().kind != CCJSqlParserTokenManager.EOF) {
377-
throw new JSQLParserException(
378-
"could only parse partial expression " + expression.toString());
379-
}
380-
} catch (ParseException ex) {
381-
throw new JSQLParserException(ex);
382-
}
383-
} catch (JSQLParserException ex1) {
384-
CCJSqlParser parser =
385-
newParser(conditionalExpressionStr).withAllowComplexParsing(true);
386-
if (consumer != null) {
387-
consumer.accept(parser);
388-
}
389-
try {
390-
expression = parser.Expression();
391-
if (!allowPartialParse
392-
&& parser.getNextToken().kind != CCJSqlParserTokenManager.EOF) {
393-
throw new JSQLParserException(
394-
"could only parse partial expression " + expression.toString());
395-
}
396-
} catch (JSQLParserException ex) {
397-
throw ex;
398-
} catch (ParseException ex) {
399-
throw new JSQLParserException(ex);
400-
}
401-
}
402-
return expression;
348+
return parseExpression(conditionalExpressionStr, allowPartialParse, consumer);
403349
}
404350

405351
/**

‎src/test/java/net/sf/jsqlparser/parser/ExpressionParsingFallbackTest.java‎

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
/*-
2+
* #%L
3+
* JSQLParser library
4+
* %%
5+
* Copyright (C) 2004 - 2026 JSQLParser
6+
* %%
7+
* Dual licensed under GNU LGPL 2.1 or Apache License 2.0
8+
* #L%
9+
*/
10+
package net.sf.jsqlparser.parser;
11+
12+
import static org.junit.jupiter.api.Assertions.assertEquals;
13+
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
14+
import static org.junit.jupiter.api.Assertions.assertNotSame;
15+
import static org.junit.jupiter.api.Assertions.assertNull;
16+
import static org.junit.jupiter.api.Assertions.assertSame;
17+
import static org.junit.jupiter.api.Assertions.assertThrows;
18+
19+
import java.util.ArrayList;
20+
import java.util.List;
21+
import java.util.function.Consumer;
22+
import net.sf.jsqlparser.JSQLParserException;
23+
import net.sf.jsqlparser.expression.Expression;
24+
import net.sf.jsqlparser.parser.feature.Feature;
25+
import org.junit.jupiter.params.ParameterizedTest;
26+
import org.junit.jupiter.params.provider.EnumSource;
27+
28+
class ExpressionParsingFallbackTest {
29+
private static final String PARTIAL_EXPRESSION = "[customer].[id] = 1 trailing tokens";
30+
31+
@ParameterizedTest
32+
@EnumSource(ParserApi.class)
33+
void configuresCompleteExpressionsOnce(ParserApi api) throws JSQLParserException {
34+
List<CCJSqlParser> attempts = new ArrayList<>();
35+
Expression result = api.parse("[customer].[id] = 1", false, parser -> {
36+
attempts.add(parser);
37+
parser.withSquareBracketQuotation(true);
38+
});
39+
40+
assertEquals("[customer].[id] = 1", result.toString());
41+
assertEquals(1, attempts.size());
42+
}
43+
44+
@ParameterizedTest
45+
@EnumSource(ParserApi.class)
46+
void reappliesConfigurationToAFreshParserForPartialInput(ParserApi api)
47+
throws JSQLParserException {
48+
List<CCJSqlParser> attempts = new ArrayList<>();
49+
List<Boolean> modes = new ArrayList<>();
50+
Expression result = api.parse(PARTIAL_EXPRESSION, true, parser -> {
51+
attempts.add(parser);
52+
modes.add(parser.getAsBoolean(Feature.allowComplexParsing));
53+
parser.withSquareBracketQuotation(true);
54+
});
55+
56+
assertEquals("[customer].[id] = 1", result.toString());
57+
assertEquals(List.of(false, true), modes);
58+
assertNotSame(attempts.get(0), attempts.get(1));
59+
}
60+
61+
@ParameterizedTest
62+
@EnumSource(ParserApi.class)
63+
void rejectsTrailingTokensWhenPartialParsingIsDisabled(ParserApi api) {
64+
JSQLParserException exception = assertThrows(JSQLParserException.class,
65+
() -> api.parse(PARTIAL_EXPRESSION, false,
66+
parser -> parser.withSquareBracketQuotation(true)));
67+
68+
assertEquals("could only parse partial expression [customer].[id] = 1",
69+
exception.getMessage());
70+
assertNull(exception.getCause());
71+
}
72+
73+
@ParameterizedTest
74+
@EnumSource(ParserApi.class)
75+
void preservesSyntaxFailureCauses(ParserApi api) {
76+
for (boolean partial : new boolean[] {false, true}) {
77+
JSQLParserException exception = assertThrows(JSQLParserException.class,
78+
() -> api.parse(";", partial, null));
79+
80+
ParseException cause = assertInstanceOf(ParseException.class, exception.getCause());
81+
assertEquals(cause.getMessage(), exception.getMessage());
82+
}
83+
}
84+
85+
@ParameterizedTest
86+
@EnumSource(ParserApi.class)
87+
void leavesNullAndEmptyInputUnconfigured(ParserApi api) throws JSQLParserException {
88+
Consumer<CCJSqlParser> unexpectedConfiguration = parser -> {
89+
throw new AssertionError("Empty input must not configure a parser");
90+
};
91+
for (boolean partial : new boolean[] {false, true}) {
92+
assertNull(api.parse(null, partial, unexpectedConfiguration));
93+
assertNull(api.parse("", partial, unexpectedConfiguration));
94+
}
95+
}
96+
97+
@ParameterizedTest
98+
@EnumSource(ParserApi.class)
99+
void acceptsNullConfiguration(ParserApi api) throws JSQLParserException {
100+
assertEquals("x = 92", api.parse("x=92", false, null).toString());
101+
assertEquals("x = 92", api.parse("x=92 trailing tokens", true, null).toString());
102+
}
103+
104+
@ParameterizedTest
105+
@EnumSource(ParserApi.class)
106+
void propagatesConfigurationFailuresWithoutRetrying(ParserApi api) {
107+
IllegalStateException failure = new IllegalStateException("Configuration failed");
108+
List<CCJSqlParser> attempts = new ArrayList<>();
109+
IllegalStateException exception = assertThrows(IllegalStateException.class,
110+
() -> api.parse("x=92", false, parser -> {
111+
attempts.add(parser);
112+
throw failure;
113+
}));
114+
115+
assertSame(failure, exception);
116+
assertEquals(1, attempts.size());
117+
}
118+
119+
private enum ParserApi {
120+
EXPRESSION, CONDITION;
121+
122+
Expression parse(String text, boolean allowPartialParse, Consumer<CCJSqlParser> consumer)
123+
throws JSQLParserException {
124+
return this == EXPRESSION
125+
? CCJSqlParserUtil.parseExpression(text, allowPartialParse, consumer)
126+
: CCJSqlParserUtil.parseCondExpression(text, allowPartialParse, consumer);
127+
}
128+
}
129+
}

0 commit comments

Comments
 (0)