Hey folks, debugging some things and noticed the following that I wanted to get a take on:
1. Some of these optimizers are eating the exceptions (eg https://github.com/eclipse/rdf4j/blob/c607df2ace72eba12d6a3f9586b7fec4b8886129/core/queryalgebra/evaluation/src/main/java/org/eclipse/rdf4j/query/algebra/evaluation/impl/ConstantOptimizer.java#L248). Do we want that? So if I pass in an unregistered function, shouldn't the exception bubble up here?
you can test it with against 4.2.1 workbench:
PREFIX fn: <http://example.com/>
SELECT ?whatever
WHERE {
BIND (fn:whatever("foo") AS ?whatever)
}
and it just silently fails.
2. should we normalize these calls to the ValueExpr derived classes so we don't have these large if instanceOf/else checks (eg https://github.com/eclipse/rdf4j/blob/7fc970aa1fe27308bb44ec20d106e56e6353fdc9/core/queryalgebra/evaluation/src/main/java/org/eclipse/rdf4j/query/algebra/evaluation/impl/StrictEvaluationStrategy.java#L919)? Looks like we would need one for evaluation/prepare if I'm reading this right. So something like:
expr.acceptEval(this, bindings) and expr.acceptPrepare(this, context)
and the derived classes would just call the appropriate evaluate/prepare through the accepted 'this' object. Would save a few cycles on the if/else checks I would think. Also a little cleaner/less casting and any new _expression_ class would need to implement it if these were made abstract in the base and less likely to break b/c we forgot to add the corresponding if/else check.
matt
