Add support for more session variables #3731 #3761 #2702 #3754
Conversation
|
Implementation is not done yet. This is just a skeleton. |
- aurora_read_replica_read_committed - group_replication_consistency - query_cache_type
renecannao
changed the title
|
retest this please |
1 similar comment
|
retest this please |
|
retest this please |
|
retest this please |
| // FIXME: but we need to switch to SQL_NAME_LAST_HIGH_WM | ||
| int idx = SQL_NAME_LAST_LOW_WM; // FIXME: here we work on absent variables | ||
| for (int i=0; i<SQL_NAME_LAST_LOW_WM; i++) { | ||
| if ( |
There was a problem hiding this comment.
Could use the errno macros here from mariadb at deps/mariadb-client/include/mysqld_error.h. So that it could read if (myerr == ER_PARSE_ERROR || myerr == ER_UNKNOWN_SYSTEM_VARIABLE || myerr == ER_QUERY_CACHE_DISABLED). These macros are safer against typos, and the names match those in the official mysql documentation for verification as well. Fewer lines of code, and self documenting.
| value1 = "0"; | ||
| } else if (strcasecmp(value1.c_str(),"on")==0 || strcasecmp(value1.c_str(),"true")==0) { | ||
| value1 = "1"; | ||
| } else if (strcasecmp(value1.c_str(),"demand")==0 || strcasecmp(value1.c_str(),"true")==0) { |
There was a problem hiding this comment.
The second or condition is already handled in the previous else if. If strcasecmp(value1.c_str(),"true")==0 condition is true, then it would be handled in the second case, always resulting in value1 = "1"; Should be removed.
| @@ -108,6 +108,19 @@ void add_value_j(std::string& j, const std::string& s, variable *v) { | |||
| } | |||
|
|
|||
|
|
|||
| void add_values_and_quotes(const std::string& name, const std::vector<std::string>& values) { | |||
| for (std::vector<std::string>::const_iterator it = values.begin(); it != values.end(); it++) { | |||
There was a problem hiding this comment.
If using iterators this way, could write it for (auto it = values.begin(); it != values.end(); it++) shorter. Since all values here are iterated, an even simpler ranged based for loop could be used for (auto& value : values) which might also save some keystrokes, in this instance, no de-referencing *it would be needed, could write value instead.
Consider renaming s to quoted_val or quoted_value for the variable name describing what the string would hold.
| vars["query_cache_type"] = new variable("query_cache_type", true, true, false); | ||
| vars["query_cache_type"]->add(bool_values); | ||
| vars["query_cache_type"]->add("2"); | ||
| add_values_and_quotes("query_cache_type", {"DeMaNd"}); |
There was a problem hiding this comment.
"demand" or "DEMAND" would be easier to read.
| @@ -70,6 +70,19 @@ std::vector<std::string> forgotten_vars {}; | |||
|
|
|||
| #include "set_testing.h" | |||
|
|
|||
| class var_counter { | |||
There was a problem hiding this comment.
For a class like this, could write struct var_counter which would is equivalent to a public access class by default, so wouldn't have to write public:. For incremental only counters, unsigned (uint64_t?) maybe better. For the constructor, could write var_counter() : count(0), unknown(0) {} and perform direct initialization.
| } | ||
| }; | ||
|
|
||
| //std::unordered_map<std::string,int> unknown_var_counters; |
There was a problem hiding this comment.
Unused comment, should remove.
| @@ -428,5 +472,8 @@ int main(int argc, char *argv[]) { | |||
| for (unsigned int i=0; i<num_threads; i++) { | |||
| pthread_join(thi[i], NULL); | |||
| } | |||
| for (std::unordered_map<std::string,var_counter>::iterator it = vars_counters.begin(); it!=vars_counters.end(); it++) { | |||
|
Automated message: PR pending admin approval for build testing |
|
retest this please |
1 similar comment
|
retest this please |
Added support for more session variables:
Closes #3731 #3761 #2702