thomasmueller commented on code in PR #535:
URL: https://github.com/apache/jackrabbit-oak/pull/535#discussion_r842916686
##########
oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineImpl.java:
##########
@@ -292,7 +290,38 @@ public Result executeQuery(
}
}
}
-
+
+ @Override
+ public Result executeQuery(
+ String statement, String language, long limit, long offset,
+ Map<String, ? extends PropertyValue> bindings,
+ Map<String, String> mappings) throws ParseException {
+ return executeQuery(statement, language, Optional.of(limit),
Optional.of(offset), bindings, mappings);
+
+ }
+
+ private static long getOffset(List<Query> queries, Optional<Long>
passedOffset) {
+ if (!passedOffset.isPresent()) {
+ return
queries.stream().map(Query::getOffset).filter(Optional::isPresent).map(Optional::get).findFirst()
+ .orElse(0L);
+ }
+ if (passedOffset.get() < 0) {
+ throw new IllegalArgumentException("Offset may not be negative,
is: " + passedOffset.get());
+ }
+ return passedOffset.get();
+ }
+
+ private static long getLimit(List<Query> queries, Optional<Long>
passedLimit) {
+ if (!passedLimit.isPresent()) {
+ return
queries.stream().map(Query::getLimit).filter(Optional::isPresent).map(Optional::get).findFirst()
Review Comment:
I would check for IllegalArgumentException as well here
##########
oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineImpl.java:
##########
@@ -292,7 +290,38 @@ public Result executeQuery(
}
}
}
-
+
+ @Override
+ public Result executeQuery(
+ String statement, String language, long limit, long offset,
+ Map<String, ? extends PropertyValue> bindings,
+ Map<String, String> mappings) throws ParseException {
+ return executeQuery(statement, language, Optional.of(limit),
Optional.of(offset), bindings, mappings);
+
+ }
+
+ private static long getOffset(List<Query> queries, Optional<Long>
passedOffset) {
+ if (!passedOffset.isPresent()) {
+ return
queries.stream().map(Query::getOffset).filter(Optional::isPresent).map(Optional::get).findFirst()
+ .orElse(0L);
+ }
+ if (passedOffset.get() < 0) {
+ throw new IllegalArgumentException("Offset may not be negative,
is: " + passedOffset.get());
+ }
+ return passedOffset.get();
+ }
+
+ private static long getLimit(List<Query> queries, Optional<Long>
passedLimit) {
+ if (!passedLimit.isPresent()) {
+ return
queries.stream().map(Query::getLimit).filter(Optional::isPresent).map(Optional::get).findFirst()
+ .orElse(Long.MAX_VALUE);
+ }
+ if (passedLimit.get() < 0) {
+ throw new IllegalArgumentException("Limit may not be negative, is:
" + passedLimit.get());
+ }
+ return passedLimit.get();
Review Comment:
I would call the getter only once... Maybe:
```
long result = passedLimit.orElse(...)
if (result < 0) {
throw new IllegalArgumentException("Limit may not be negative, is: " +
result);
}
return result;
```
--
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]
