Github user ilooner commented on the issue:
https://github.com/apache/drill/pull/923
So it looks like Drill's treatment of OptionType and OptionScope is
inconsistent at present. To correct this I am doing the following:
- OptionType seems to be I'll defined. I think the intention is that it
defines the levels in which an option can be configured. This should be a
static property tied to an option (much like a validator). In reality it looks
like the OptionType of an option can be changed depending on where it is set,
which is incorrect behavior. I think this came about because it is the user's
responsibility to define the OptionType for an option value which is in correct.
- OptionScope will define the level at which the option was set
To solidify the semantics I am doing the following:
- OptionManagers no longer allow an option value to be set. Only the
following methods are exposed: ```
void setLocalOption(String name, boolean value);
void setLocalOption(String name, long value);
void setLocalOption(String name, double value);
void setLocalOption(String name, String value);
void setLocalOption(OptionValue.Kind kind, String name, String value);
``` Doing this prevents the possibility of defining an incorrect scope or
type. The option type is now determined by the static option definition. The
option scope is determined by the option manager on which the option was set.
- I have also cleaned up the OptionManager interface so that it does not
require the option type to be passed to the delete methods. This helps clean up
the semantics further because OptionType was being overloaded to define both
where an option can be set and which option manager can set it. This is unclear
because some options can be set exclusively at one level (e.g. only the session
level) whereas other options can be set at multiple levels (e.g. system and
session levels). It also introduces the possibility for error by allowing the
user to provide the wrong type to the wrong option manager.
---
