Performance Improvements #1439
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,12 @@ | |
| import java.io.IOException; | ||
| import java.io.InputStream; | ||
| import java.io.Reader; | ||
| import java.util.concurrent.Callable; | ||
| import java.util.concurrent.ExecutorService; | ||
| import java.util.concurrent.Executors; | ||
| import java.util.concurrent.Future; | ||
| import java.util.concurrent.TimeUnit; | ||
| import java.util.concurrent.TimeoutException; | ||
| import java.util.function.Consumer; | ||
| import net.sf.jsqlparser.JSQLParserException; | ||
| import net.sf.jsqlparser.expression.Expression; | ||
|
|
@@ -23,8 +29,11 @@ | |
| * | ||
| * @author toben | ||
| */ | ||
|
|
||
| @SuppressWarnings("PMD.CyclomaticComplexity") | ||
| public final class CCJSqlParserUtil { | ||
| public final static int ALLOWED_NESTING_DEPTH = 10; | ||
| public static final int PARSER_TIMEOUT = 6000; | ||
|
|
||
| private CCJSqlParserUtil() { | ||
| } | ||
|
|
@@ -54,13 +63,25 @@ public static Statement parse(String sql) throws JSQLParserException { | |
| * @throws JSQLParserException | ||
| */ | ||
| public static Statement parse(String sql, Consumer<CCJSqlParser> consumer) throws JSQLParserException { | ||
| Statement statement = null; | ||
|
|
||
| // first, try to parse fast and simple | ||
| try { | ||
| CCJSqlParser parser = newParser(sql).withAllowComplexParsing(false); | ||
|
There was a problem hiding this comment. If complex parsing is initially switched off, it should not be switched on in this method. Right? However, this is only some kind of hack. The grammar itself is flawed if we need to do something like this. The porblem is the excessive use of Lookups, but if the grammar would be designed that a fixed lookup would be enough, then we do not need a switch like this. There was a problem hiding this comment. You are correct in your observation, but I respectfully do not share your conclusion:
Best illustration is issue #1444: select doc.fullName from XWikiDocument doc where doc.fullName like :fullName escape '/' and doc.author like :author escape '!'Is this select doc.fullName from XWikiDocument doc where ( doc.fullName like :fullName escape '/' )
and ( doc.author like :author escape '!' )Or is this select doc.fullName from XWikiDocument doc where doc.fullName like :fullName escape ( '/'
and doc.author like :author escape '!' )JavaCC by its nature lacks a lot of functionality to deal with those requirements. JavaCC 21 added some features for a reason and I believe ANTLR is there for a reason too. Long story short: So the suggest approach, while not a beauty at all solves that in a pragmatic way: There was a problem hiding this comment. Of course, if anyone can point me on how to avoid those |
||
| if (consumer != null) { | ||
| consumer.accept(parser); | ||
| } | ||
| statement = parseStatement(parser); | ||
| } catch (JSQLParserException ex) { | ||
| if (getNestingDepth(sql)<=ALLOWED_NESTING_DEPTH) { | ||
| CCJSqlParser parser = newParser(sql).withAllowComplexParsing(true); | ||
| if (consumer != null) { | ||
| consumer.accept(parser); | ||
| } | ||
| statement = parseStatement(parser); | ||
| } | ||
| } | ||
| return statement; | ||
| } | ||
|
|
||
| public static CCJSqlParser newParser(String sql) { | ||
|
|
@@ -112,24 +133,44 @@ public static Expression parseExpression(String expression, boolean allowPartial | |
| }); | ||
| } | ||
|
|
||
| @SuppressWarnings("PMD.CyclomaticComplexity") | ||
| public static Expression parseExpression(String expressionStr, boolean allowPartialParse, Consumer<CCJSqlParser> consumer) throws JSQLParserException { | ||
| Expression expression = null; | ||
|
|
||
| // first, try to parse fast and simple | ||
| try { | ||
| CCJSqlParser parser = newParser(expressionStr).withAllowComplexParsing(false); | ||
| if (consumer != null) { | ||
| consumer.accept(parser); | ||
| } | ||
| try { | ||
| expression = parser.Expression(); | ||
| if (parser.getNextToken().kind != CCJSqlParserTokenManager.EOF) { | ||
| throw new JSQLParserException("could only parse partial expression " + expression.toString()); | ||
| } | ||
| } catch (ParseException ex) { | ||
| throw new JSQLParserException(ex); | ||
| } | ||
| } catch (JSQLParserException ex1) { | ||
| // when fast simple parsing fails, try complex parsing but only if it has a chance to succeed | ||
| if (getNestingDepth(expressionStr)<=ALLOWED_NESTING_DEPTH) { | ||
| CCJSqlParser parser = newParser(expressionStr).withAllowComplexParsing(true); | ||
| if (consumer != null) { | ||
| consumer.accept(parser); | ||
| } | ||
| try { | ||
| expression = parser.Expression(); | ||
| if (!allowPartialParse && parser.getNextToken().kind != CCJSqlParserTokenManager.EOF) { | ||
| throw new JSQLParserException("could only parse partial expression " + expression.toString()); | ||
| } | ||
| } catch (JSQLParserException ex) { | ||
| throw ex; | ||
| } catch (ParseException ex) { | ||
| throw new JSQLParserException(ex); | ||
| } | ||
| } | ||
| } | ||
| return expression; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -158,24 +199,43 @@ public static Expression parseCondExpression(String condExpr, boolean allowParti | |
| }); | ||
| } | ||
|
|
||
| @SuppressWarnings("PMD.CyclomaticComplexity") | ||
| public static Expression parseCondExpression(String conditionalExpressionStr, boolean allowPartialParse, Consumer<CCJSqlParser> consumer) throws JSQLParserException { | ||
| Expression expression = null; | ||
|
|
||
| // first, try to parse fast and simple | ||
| try { | ||
| CCJSqlParser parser = newParser(conditionalExpressionStr).withAllowComplexParsing(false); | ||
| if (consumer != null) { | ||
| consumer.accept(parser); | ||
| } | ||
| try { | ||
| expression = parser.Expression(); | ||
| if (parser.getNextToken().kind != CCJSqlParserTokenManager.EOF) { | ||
| throw new JSQLParserException("could only parse partial expression " + expression.toString()); | ||
| } | ||
| } catch (ParseException ex) { | ||
| throw new JSQLParserException(ex); | ||
| } | ||
| } catch (JSQLParserException ex1) { | ||
| if (getNestingDepth(conditionalExpressionStr)<=ALLOWED_NESTING_DEPTH) { | ||
| CCJSqlParser parser = newParser(conditionalExpressionStr).withAllowComplexParsing(true); | ||
| if (consumer != null) { | ||
| consumer.accept(parser); | ||
| } | ||
| try { | ||
| expression = parser.Expression(); | ||
| if (!allowPartialParse && parser.getNextToken().kind != CCJSqlParserTokenManager.EOF) { | ||
| throw new JSQLParserException("could only parse partial expression " + expression.toString()); | ||
| } | ||
| } catch (JSQLParserException ex) { | ||
| throw ex; | ||
| } catch (ParseException ex) { | ||
| throw new JSQLParserException(ex); | ||
| } | ||
| } | ||
| } | ||
| return expression; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -184,11 +244,25 @@ public static Expression parseCondExpression(String condExpr, boolean allowParti | |
| * @throws JSQLParserException | ||
| */ | ||
| public static Statement parseStatement(CCJSqlParser parser) throws JSQLParserException { | ||
| Statement statement = null; | ||
| try { | ||
| ExecutorService executorService = Executors.newSingleThreadExecutor(); | ||
| Future<Statement> future = executorService.submit(new Callable<Statement>() { | ||
| @Override | ||
| public Statement call() throws Exception { | ||
| return parser.Statement(); | ||
| } | ||
| }); | ||
| executorService.shutdown(); | ||
|
|
||
| statement = future.get(PARSER_TIMEOUT, TimeUnit.MILLISECONDS); | ||
| } catch (TimeoutException ex) { | ||
| parser.interrupted = true; | ||
| throw new JSQLParserException("Time out occurred.", ex); | ||
| } catch (Exception ex) { | ||
| throw new JSQLParserException(ex); | ||
| } | ||
| return statement; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -197,10 +271,20 @@ public static Statement parseStatement(CCJSqlParser parser) throws JSQLParserExc | |
| * @return the statements parsed | ||
| */ | ||
| public static Statements parseStatements(String sqls) throws JSQLParserException { | ||
| Statements statements = null; | ||
|
|
||
| // first, try to parse fast and simple | ||
| try { | ||
| CCJSqlParser parser = newParser(sqls).withAllowComplexParsing(false); | ||
| statements = parseStatements(parser); | ||
| } catch (JSQLParserException ex) { | ||
| // when fast simple parsing fails, try complex parsing but only if it has a chance to succeed | ||
| if (getNestingDepth(sqls)<=ALLOWED_NESTING_DEPTH) { | ||
| CCJSqlParser parser = newParser(sqls).withAllowComplexParsing(true); | ||
| statements = parseStatements(parser); | ||
| } | ||
| } | ||
| return statements; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -209,11 +293,25 @@ public static Statements parseStatements(String sqls) throws JSQLParserException | |
| * @throws JSQLParserException | ||
| */ | ||
| public static Statements parseStatements(CCJSqlParser parser) throws JSQLParserException { | ||
| Statements statements = null; | ||
| try { | ||
| ExecutorService executorService = Executors.newSingleThreadExecutor(); | ||
| Future<Statements> future = executorService.submit(new Callable<Statements>() { | ||
| @Override | ||
| public Statements call() throws Exception { | ||
| return parser.Statements(); | ||
| } | ||
| }); | ||
| executorService.shutdown(); | ||
|
|
||
| statements = future.get(PARSER_TIMEOUT, TimeUnit.MILLISECONDS); | ||
| } catch (TimeoutException ex) { | ||
| parser.interrupted = true; | ||
| throw new JSQLParserException("Time out occurred.", ex); | ||
| } catch (Exception ex) { | ||
| throw new JSQLParserException(ex); | ||
| } | ||
| return statements; | ||
| } | ||
|
|
||
| public static void streamStatements(StatementListener listener, InputStream is, String encoding) throws JSQLParserException { | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just curious. Does this work the same way maven dependency versioning works like [1.0,2.0)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not know about Maven does (In fact I hate Maven. You can't even
cleana project while offline. I went straight from ANT to Gradle, which "just understands and does what I want" instead of forcing me into any dogma). Thislatest.releaseautomatically retrieves the Latest Release of a Gradle Plugin.