Reconsider ExpressionContext API for accessing QuerySettings instance

    • Type: Improvement
    • Resolution: Unresolved
    • Priority: Minor - P4
    • None
    • Affects Version/s: None
    • Component/s: None
    • None
    • Query Execution
    • None
    • 3
    • TBD
    • None
    • None
    • None
    • None
    • None
    • None
    • None

      Currently _querySettings instance on the ExpressionContext is defined as boost:optional and !_querySettings.has_value() means that the settings haven't been looked up and aren't currently initialised. However, for ExpressionContext users we use empty settings (query_settings::QuerySettings()) to indicate that there are no settings. We also provide a pair of getter/setter methods which encapsulate this semantics.

      All works well with this API until there is a need to read and modify the boost:optional value of the _querySettings. One specific use case for this is SERVER-108210 where we want to move ExpressionContextBuilder into its own library and separate it from the ExpressionContext itself. As part of this change ExpressionContext::copyWith also has to be moved into a standalone function. The caveat is, copyWith as a member function, can modify private members of the ExpressionContext, including _querySettings, by directly accessing it. Once its re-implemented as a free function, it will only be able to obtain and set the QuerySettings value on a new copy through the public getter/setter methods, which has a different semantics than accessing _querySettings directly. There are some workarounds to deal with this issues (e.g., make the free copy function a friend of the ExpressionContext, or expose a pair of getter/setter method for the vanilla boost::optional value) but none of them are perfect.

      This ticket is a placeholder to try to re-consider the exiting API for accessing QuerySettings instance to avoid shortcomings of those listed workarounds.

              Assignee:
              Unassigned
              Reporter:
              Anton Korshunov
              Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

                Created:
                Updated: