thomasmueller commented on a change in pull request #446:
URL: https://github.com/apache/jackrabbit-oak/pull/446#discussion_r774398222
##########
File path:
oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineSettings.java
##########
@@ -93,6 +93,22 @@
private String[] classNamesIgnoredInCallTrace = new String[] {};
+
+ private static final String OAK_QUERY_LENGTH_WARN_LIMIT =
"oak.query.length.warn.limit";
+ private static final String OAK_QUERY_LENGTH_ERROR_LIMIT =
"oak.query.length.error.limit";
+
+ private final long queryLengthWarnLimit =
Long.getLong(OAK_QUERY_LENGTH_WARN_LIMIT, 102400); // 100 KB
Review comment:
So let's use "1024 * 1024" here (1 MiB)
##########
File path:
oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineImpl.java
##########
@@ -151,12 +151,19 @@
} else {
LOG.debug("Parsing {} statement: {}", language, statement);
}
+ QueryEngineSettings settings = context.getSettings();
+ if (statement.length() > (settings.getQueryLengthErrorLimit())){
+ LOG.error("Too large query: " + statement);
+ throw new RuntimeException("Query length "+ statement.length() + "
is larger than max supported query length: " +
settings.getQueryLengthErrorLimit());
Review comment:
Maybe use ParseException instead of RuntimeException? We could use
another exception as well, e.g. IllegalArgumentException, but we use
ParseException below as well; I think it's better than to introduce a new
exception type.
##########
File path:
oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineSettings.java
##########
@@ -93,6 +93,22 @@
private String[] classNamesIgnoredInCallTrace = new String[] {};
+
+ private static final String OAK_QUERY_LENGTH_WARN_LIMIT =
"oak.query.length.warn.limit";
+ private static final String OAK_QUERY_LENGTH_ERROR_LIMIT =
"oak.query.length.error.limit";
+
+ private final long queryLengthWarnLimit =
Long.getLong(OAK_QUERY_LENGTH_WARN_LIMIT, 102400); // 100 KB
+ private final long queryLengthErrorLimit =
Long.getLong(OAK_QUERY_LENGTH_ERROR_LIMIT, 1048576); //1MB (1024KB)
Review comment:
We currently want to see if there are warnings, and not throw errors, so
100 * 1024 * 1024 would be better I think (100 MiB). BTW did you know that
newer Java versions supports "_" in numbers, so 1_000_000 would be 1 million.
But I think using 100 * 1024 is OK (serves the same purpose).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
