thomasmueller commented on code in PR #535:
URL: https://github.com/apache/jackrabbit-oak/pull/535#discussion_r842448019
##########
oak-core/src/main/java/org/apache/jackrabbit/oak/query/SQL2Parser.java:
##########
@@ -290,6 +294,14 @@ private SelectorImpl parseSelector() throws ParseException
{
return factory.selector(nodeTypeInfo, selectorName);
}
+
+ private long readBase10WholeNumber() throws ParseException {
+ String label = readName();
+ if (!label.matches("\\d*") || label.isEmpty() || label.length() > 19
/* Length of MAX_LONG */) {
Review Comment:
I don't think the "if" condition is needed; instead I would use catch
(NumberFormatException nfe) (same as below).
##########
oak-core/src/main/java/org/apache/jackrabbit/oak/query/xpath/Statement.java:
##########
@@ -376,6 +376,21 @@ private static void appendQueryOptions(StringBuilder buff,
QueryOptions queryOpt
buff.append("]");
optionCount++;
}
+ if (queryOptions.offset != -1) {
+ if (optionCount > 0) {
+ buff.append(", ");
+ }
+ buff.append("offset ");
+ buff.append(queryOptions.offset);
+ optionCount++;
+ }
+ if (queryOptions.limit != -1) {
+ if (optionCount > 0) {
+ buff.append(", ");
+ }
+ buff.append("limit ");
+ buff.append(queryOptions.limit);
Review Comment:
missing optionCount++... I know, it's not needed here really, but it's easy
to forget to add it later...
Actually, it's probably better to add the options first into an
ArrayList<String>, and then use String.join(", ", list)...
##########
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 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 long getLimit(List<Query> queries, Optional<Long> passedLimit) {
Review Comment:
static?
##########
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 long getOffset(List<Query> queries, Optional<Long> passedOffset) {
Review Comment:
(I didn't compile this sorry) but could it be static?
##########
oak-core/src/main/java/org/apache/jackrabbit/oak/query/SQL2Parser.java:
##########
@@ -290,6 +294,14 @@ private SelectorImpl parseSelector() throws ParseException
{
return factory.selector(nodeTypeInfo, selectorName);
}
+
+ private long readBase10WholeNumber() throws ParseException {
Review Comment:
I would rename to readNumber()
--
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]
