-
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.
- is related to
-
SERVER-108210 Move ExpressionContextBuilder into a separate library
-
- Closed
-