[Feature] Implement Visitor Design Pattern in QueryBuilder

[vibrantvarun]

Is your feature request related to a problem? Please describe.
A way for developer to get the specific child query builder object from the nested QueryBuilder. For example: If a user passes a nested query, then it will be converted to a QueryBuilder object. That QueryBuilder object has some more nested querybuilders wrapped in it. There should be a way to identify and fetch the specific query builder object from the generic query builder object.

Describe the solution you'd like
Lucene has a visit method in an abstract class Query.java & QueryVisitor.java. This visit method gives power to all the classes that extend Query.java to determine the right object type (like BoolQuery, FuzzyQuery etc) from the argument passed in the visit method. Similarly, there is a need to implement the same visit method in QueryBuilder.java in OpenSearch

Additional context
If we are able build this feature then it will empower the search pipelines as then a developer will have a capability to identify the right query builder and update it as per the need.

[navneet1v]

I would be really interested to see this feature available in QueryBuilders. Currently with SearchRequestProcessors you can only change the outer most elements of query like size, from , to parameters but if we have this feature available the utility of SearchRequestProcessors will increase a lot.

Thanks @vibrantvarun for creating this github issue.

cc: @msfroh

[msfroh]

Yeah -- I think visitors in QueryBuilder would be really useful. I have a task to write up shortly on query rewriting to improve caching of required clauses that would also benefit from a visitor (to find/collect required clauses) -- though I guess it could also be done using the Lucene visitor.

One thing I would do differently compared to the Lucene visitor is go for an API more like:

interface QueryBuilderVisitor {
  void visit(QueryBuilder qb);
  QueryBuilderVisitor getChildVisitor(BooleanClause.Occur occur);
}

Then I would recommend that QueryBuilders perform pre-order traversal. As a sanity check of the API, it should be easy for a visitor to render a tree view of a query, IMO.

[vibrantvarun]

@msfroh what is the significance of having getChildVisitor Method?

[msfroh]

To provide a bit more detail, I would suggest that BoolQueryBuilder's visit method should look like:

    public void visit(QueryBuilderVisitor visitor) {
        visitor.visit(this);
        if (mustClauses.isEmpty() == false) {
            QueryBuilderVisitor subVisitor = visitor.getChildVisitor(Occur.MUST);
            for (QueryBuilder mustClause : mustClauses) {
                mustClause.visit(subVisitor);
            }
        }
        if (shouldClauses.isEmpty() == false) {
            QueryBuilderVisitor subVisitor = visitor.getChildVisitor(Occur.SHOULD);
            for (QueryBuilder shouldClause : shouldClauses) {
                shouldClause.visit(subVisitor);
            }
        }
        if (mustNotClauses.isEmpty() == false) {
            QueryBuilderVisitor subVisitor = visitor.getChildVisitor(Occur.MUST_NOT);
            for (QueryBuilder mustNotClause : mustNotClauses) {
                mustNotClause.visit(subVisitor);
            }
        }
        if (filterClauses.isEmpty() == false) {
            QueryBuilderVisitor subVisitor = visitor.getChildVisitor(Occur.FILTER);
            for (QueryBuilder filterClause : filterClauses) {
                filterClause.visit(subVisitor);
            }
        }
    }

That way, the visitor is told that we're going down the tree (i.e. the next thing visited is under the current query).

A visitor that doesn't care about the overall query structure can just return this, to let that single instance visit all nodes of the tree.

A visitor that does care about the structure of the tree can return a new visitor with the awareness that it's visiting the children. Also, some visitors only care about some types of subtree -- e.g. required clauses versus SHOULD clauses.

[vibrantvarun]

Closing the issue as PR's are merged now.