Investigate how to have Visitors consider extension stages in their implementation

XMLWordPrintableJSON

    • Query Integration
    • None
    • None
    • None
    • None
    • None
    • None
    • None

      For full context on this task, refer back to this thread which prompted the original discussion.

      Within the query code base, we use the Visitor pattern extensively to perform double dispatch on DocumentSource implementations. This poses a problem for extensions, because we don't have a way to provide callbacks into the extension for implementing the visit() override for extension stages. It's possible we could implement the visitors via a set of properties on the stages. Whether or not we ultimately end up supporting visitor overrides in extensions, what we must definitely prevent is extensions stages not being considered at all in existing Visitor implementations, or new Visitors. Even if a Visitor override is not supported for extensions, it should be carefully considered every time a Visitor is implemented.

      Task 1: How do we ensure that new & existing Visitors in to the code base which operate on DocumentSource consider DocumentSourceExtensionOptimizable when they are implemented?

      Generally, with a typical Visitor, we could draw attention to DocumentSourceExtensionOptimizable by providing the pure virtual method:

      BaseVisitor::visit(const DocumentSourceExtensionOptimizable&) = 0; 

      This would trigger a compilation failure if any Visitor implementation does not provide a specific override for extensions.

      However, this solution is not viable with the current implementation we have in the server. 

      In order to accommodate DocumentSources implemented outside of the server (for example, in the enterprise module), we provide a "dynamic" Visitor implementation, which leverages a visitor function registry (see document_source_visitor_registry.h). 

      We use the templated function registerVisitFuncs to register a function pointer using a specialization of DocumentSourceContextVisitorBase and DocumentSource as the key. It's important to note, that this is facilitated by defining a template function helper which is parametrized on both the context and the document source type defined in the registry header. This templated helper is what is used as the function pointer when registering the visitor function.

      This templated helper then calls visit on the concrete templated types completing the double dispatch.  It is up to the visitor's implementer to make sure that a free function is provided for each DocumentSource specialization. 

      In order to force implementers to consider DocumentSourceExtensionOptimizable at compile time, we can make this modification to force implementers to implement a free function visitExtensionStage. See PR

      As part of this ticket, we want to provide this mechanism, to guarantee that extension stages are guaranteed to considered in Visitor implementations, even if it's just a default/no-op. This will raise awareness around potential limitations of extension stages in participating with a specific visitor, or requirements for accommodating the visitor logic in the API.

      Task 2: 

      Once we identify any all existing Visitors in the server operating on DocumentSources, we should figure out if we want to provide support for those visitors in extension stages. If it makes sense to do so, we should investigate whether or not it is possible to model those visitor overrides as properties, or if a callback into the extension is feasible. This task will be useful in helping us define a way to deal with implementing visitor logic across the API boundary, as this will become increasingly important as we aim to support pipeline optimizations using the Visitor pattern. 

      As part of this task, also document how we intend to handle adding support for new Visitors in the Extensions API. This would likely be done via a minor version bump in most cases, assuming the participation in the visitor is optional and not a breaking change.

            Assignee:
            Unassigned
            Reporter:
            Santiago Roche
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

              Created:
              Updated: