[rdf4j-dev] Visitor pattern or ...

[jerven Bolleman <jerven.bolleman@sib.swiss>]

Hi Matt, Jeen,

> 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:

So what you are proposing is a classic visitor pattern and I have 
thought about this a lot. But in the end I thought not to go that way 
right now. Because I think when pattern matching comes we will want to 
use that instead. Probably with a sealed interface for all algebra members.

Also in 5.x I want to get rid of the evaluate methods and have only prepare.

So it felt like a lot of code churn for no long term benefit. Or not 
enough benefit for me to be the one to do it ;) A second issue is that 
it set's how something should be evaluated in the algebra. While I think
the evaluation choice is independent of the algebra. e.g. hash join/or 
merge sorted join is an evaluation choice.

https://docs.oracle.com/en/java/javase/19/language/sealed-classes-and-interfaces.html

https://docs.oracle.com/en/java/javase/19/language/pattern-matching-switch-expressions-and-statements.html#GUID-E69EEA63-E204-41B4-AA7F-D58B26A3B232

Regards,
Jerven

On 23/11/2022 15:29, Matthew Nguyen via rdf4j-dev wrote:
> 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
>
> _______________________________________________
> rdf4j-dev mailing list
> rdf4j-dev@xxxxxxxxxxx
> To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/rdf4j-dev

--

	*Jerven Tjalling Bolleman*
Principal Software Developer
*SIB | Swiss Institute of Bioinformatics*
1, rue Michel Servet - CH 1211 Geneva 4 - Switzerland
t +41 22 379 58 85
Jerven.Bolleman@sib.swiss - www.sib.swiss