adamcin commented on code in PR #535: URL: https://github.com/apache/jackrabbit-oak/pull/535#discussion_r861844755
########## oak-api/src/main/java/org/apache/jackrabbit/oak/api/QueryEngine.java: ########## @@ -91,6 +92,24 @@ Result executeQuery( String statement, String language, long limit, long offset, Map<String, ? extends PropertyValue> bindings, Map<String, String> mappings) throws ParseException; + + /** + * Execute a query and get the result. + * + * @param statement the query statement + * @param language the language + * @param limit the maximum result set size (may not be negative but may be empty) + * @param offset the number of rows to skip (may not be negative but may be empty) + * @param bindings the bind variable value bindings + * @param mappings namespace prefix mappings + * @return the result + * @throws ParseException if the statement could not be parsed + * @throws IllegalArgumentException if there was an error executing the query + */ + Result executeQuery( + String statement, String language, Optional<Long> limit, Optional<Long> offset, Review Comment: Just a drive-by comment here, but instead of `Optional<Long>`, could you define a `Limit` type to serve as the wrapper for the limit long, and an `Offset` type to serve as the wrapper for the offset long? Then you would also eliminate the potential for callers accidentally reversing the order of the arguments. Give both types factory methods like `.of(long)` and `.none()` and an instance method `Optional<Long> toOptional()`, or `boolean isPresent()` and `long getValue()`. -- 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: dev-unsubscr...@jackrabbit.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org