Hi Brian, sorry for a delay, I'm a bit overwhelmed at the moment and at times like these I usually focus on issues I can deal with fast
thanks for a patch. I have to admit I'm slightly hesitant at the moment as to whether I want to apply it or not.
First, please do run 'mvn -Psetup.eclipse' which should ensure you get all the formatting right and will make it easier for me to deal with the patch. We don't usually have the authors tags but we do acknowledge the authors in the commit emails.
As far as the actual idea about introducing a visitor is concerned.
I do agree that SearchCondition.toSQL will not work in all the cases even when some complex enough SQL expressions have to be produced. I'd like to keep toSQL as not deprecated because I feel it will work in 80% well (80% will want SQL on the output and 80% will work with simple enough queries for toSQL to work properly) and it is simpler for a user then worry about initiating a visitor class.
Now, I'm open to changing the SeachCondition interface to support a visitor pattern, may be for 2.4, as it was difficult to come with the right interface without having practitioners like yourself trying it.
However, I did expect toSQL() won't work in 100% cases so that is why SearchCondition#getSearchConditions() exists.
Giving this method and SearchCondition.getType(), one can 'pull' all the conditions and easily build the custom expression.
Can you please let me know why do you think applying a Visitor pattern is better ? The 'pull' style kind of appeals better to me given that it is obvious when the top level condition is checked and thus the start of the expression like 'SELECT' can be added. As a side note I'd prefer to have 2 methods only in the Visitor interface, if we were to go for it (one method accepting the base SearchCondition interface with implementations relying on getType() and an explicit toExpression())