Merge pull request #4445 from yandex/send_logs_level_fix

Fixed accidential incompatibility due to wrong implementation of "send_logs_level" setting

dbms/programs/server/TCPHandler.cpp

@@ -164,10 +164,10 @@ void TCPHandler::runImpl()
 
             /// Should we send internal logs to client?
             if (client_revision >= DBMS_MIN_REVISION_WITH_SERVER_LOGS
-                && query_context.getSettingsRef().send_logs_level.value != "none")
+                && query_context.getSettingsRef().send_logs_level.value != LogsLevel::none)
             {
                 state.logs_queue = std::make_shared<InternalTextLogsQueue>();
-                state.logs_queue->max_priority = Poco::Logger::parseLevel(query_context.getSettingsRef().send_logs_level.value);
+                state.logs_queue->max_priority = Poco::Logger::parseLevel(query_context.getSettingsRef().send_logs_level.toString());
                 CurrentThread::attachInternalTextLogsQueue(state.logs_queue);
             }
 

dbms/src/Interpreters/Settings.h

@@ -280,7 +280,7 @@ struct Settings
     M(SettingBool, log_profile_events, true, "Log query performance statistics into the query_log and query_thread_log.") \
     M(SettingBool, log_query_settings, true, "Log query settings into the query_log.") \
     M(SettingBool, log_query_threads, true, "Log query threads into system.query_thread_log table. This setting have effect only when 'log_queries' is true.") \
-    M(SettingLogsLevel, send_logs_level, "none", "Send server text logs with specified minimum level to client. Valid values: 'trace', 'debug', 'information', 'warning', 'error', 'none'") \
+    M(SettingLogsLevel, send_logs_level, LogsLevel::none, "Send server text logs with specified minimum level to client. Valid values: 'trace', 'debug', 'information', 'warning', 'error', 'none'") \
     M(SettingBool, enable_optimize_predicate_expression, 0, "If it is set to true, optimize predicates to subqueries.") \
     \
     M(SettingUInt64, low_cardinality_max_dictionary_size, 8192, "Maximum size (in rows) of shared global dictionary for LowCardinality type.") \

dbms/src/Interpreters/SettingsCommon.cpp

@@ -623,39 +623,39 @@ void SettingDateTimeInputFormat::write(WriteBuffer & buf) const
 }
 
 
-const std::vector<String> SettingLogsLevel::log_levels =
+SettingLogsLevel::Value SettingLogsLevel::getValue(const String & s)
 {
-        "none",
-        "trace",
-        "debug",
-        "information",
-        "warning",
-        "error"
-};
+    if (s == "none") return Value::none;
+    if (s == "error") return Value::error;
+    if (s == "warning") return Value::warning;
+    if (s == "information") return Value::information;
+    if (s == "debug") return Value::debug;
+    if (s == "trace") return Value::trace;
 
+    throw Exception("Unknown logs level: '" + s + "', must be one of: none, error, warning, information, debug, trace", ErrorCodes::BAD_ARGUMENTS);
+}
 
-SettingLogsLevel::SettingLogsLevel(const String & level)
+String SettingLogsLevel::toString() const
 {
-    set(level);
+    const char * strings[] = {"none", "error", "warning", "information", "debug", "trace"};
+    return strings[static_cast<size_t>(value)];
 }
 
-
-void SettingLogsLevel::set(const String & level)
+void SettingLogsLevel::set(Value x)
 {
-    auto it = std::find(log_levels.begin(), log_levels.end(), level);
-    if (it == log_levels.end())
-        throw Exception("Log level '" + level + "' not allowed.", ErrorCodes::UNKNOWN_LOG_LEVEL);
-
-    value = *it;
+    value = x;
     changed = true;
 }
 
-
-void SettingLogsLevel::set(const Field & level)
+void SettingLogsLevel::set(const Field & x)
 {
-    set(safeGet<String>(level));
+    set(safeGet<const String &>(x));
 }
 
+void SettingLogsLevel::set(const String & x)
+{
+    set(getValue(x));
+}
 
 void SettingLogsLevel::set(ReadBuffer & buf)
 {
@@ -664,13 +664,6 @@ void SettingLogsLevel::set(ReadBuffer & buf)
     set(x);
 }
 
-
-String SettingLogsLevel::toString() const
-{
-    return value;
-}
-
-
 void SettingLogsLevel::write(WriteBuffer & buf) const
 {
     writeBinary(toString(), buf);

dbms/src/Interpreters/SettingsCommon.h

@@ -157,6 +157,7 @@ struct SettingFloat
 };
 
 
+/// TODO: X macro
 enum class LoadBalancing
 {
     /// among replicas with a minimum number of errors selected randomly
@@ -381,24 +382,38 @@ struct SettingDateTimeInputFormat
 };
 
 
+enum class LogsLevel
+{
+    none = 0,    /// Disable
+    error,
+    warning,
+    information,
+    debug,
+    trace,
+};
+
 class SettingLogsLevel
 {
 public:
+    using Value = LogsLevel;
 
-    String value;
+    Value value;
     bool changed = false;
-    static const std::vector<String> log_levels;
 
-    SettingLogsLevel(const String & level);
-    operator String() const { return value; }
-    void set(const String & level);
-    void set(const Field & level);
-    void set(ReadBuffer & buf);
+    SettingLogsLevel(Value x) : value(x) {}
+
+    operator Value() const { return value; }
+    SettingLogsLevel & operator= (Value x) { set(x); return *this; }
+
+    static Value getValue(const String & s);
 
     String toString() const;
+
+    void set(Value x);
+    void set(const Field & x);
+    void set(const String & x);
+    void set(ReadBuffer & buf);
     void write(WriteBuffer & buf) const;
 };
 
-
-
 }