fix(test/tap): operator-precedence logic bug in set_testing-{,multi-,240-}t.cpp

All three files had the same incorrectly-grouped failure check at the
heart of their variable-comparison loop. The intended semantics were:

  if special_sqlmode is true AND verified_special_sqlmode is false, fail
  or if k == mysql_vars.end(),                                     fail
  or if s == proxysql_vars["conn"].end(),                          fail
  or if special_sqlmode is false AND the inner disjunction holds,  fail

where the inner disjunction is

  (el.key() != "session_track_gtids" && values differ)
  OR
  (el.key() == "session_track_gtids" && check_session_track_gtids(...) fails)

Because C++ '&&' binds tighter than '||', the source

  (special_sqlmode == false &&
     (el.key() != "session_track_gtids" && (...)) ||
     (el.key() == "session_track_gtids" && !check_session_track_gtids(...))
  )

actually parsed as

  ( special_sqlmode == false && (el.key() != "session_track_gtids" && (...)) )
  ||
  (el.key() == "session_track_gtids" && !check_session_track_gtids(...))

i.e. the 'session_track_gtids' branch was promoted to a standalone
failure condition that fired even when special_sqlmode == true (or in
set_testing-240-t.cpp, also when parsing_optimizer_switch == true).
This caused tests to flag spurious failures whenever session_track_gtids
disagreed in the "we intentionally don't care about this iteration"
cases.

-Wparentheses flagged all three files. Fix is one extra pair of
parentheses around the inner disjunction so it's truly guarded by the
outer special_sqlmode / parsing_optimizer_switch condition.

Compile-verified with -fsyntax-only -Wall on all three files.
lint-tap-tests-static-analysis
Rene Cannao 2 months ago
parent 20036eb54e
commit a6b8afa1f2

@ -409,15 +409,21 @@ void * my_conn_thread(void *arg) {
if (std::find(possible_unknown_variables.begin(), possible_unknown_variables.end(), el.key()) != possible_unknown_variables.end()) {
vars_counters[el.key()].count++;
}
// The inner disjunction over session_track_gtids handling must
// be grouped so it only fires when special_sqlmode==false and
// parsing_optimizer_switch==false. Without the extra outer
// parentheses, operator precedence treated the
// 'el.key() == "session_track_gtids"' branch as a standalone
// failure condition that fired even in the other cases.
if (
(special_sqlmode == true && verified_special_sqlmode == false) ||
(k == mysql_vars.end()) ||
(s == proxysql_vars["conn"].end()) ||
( (parsing_optimizer_switch == true) && (optimizer_switch_matches == false) ) ||
(special_sqlmode == false && parsing_optimizer_switch == false &&
(special_sqlmode == false && parsing_optimizer_switch == false && (
(el.key() != "session_track_gtids" && (k.value() != el.value() || s.value() != el.value())) ||
(el.key() == "session_track_gtids" && !check_session_track_gtids(el.value(), s.value(), k.value()))
)
))
) {
if ( k != mysql_vars.end() && s != proxysql_vars["conn"].end()) {
if (k.value() == UNKNOWNVAR) { // mysql doesn't recognize the variable

@ -274,12 +274,18 @@ void * my_conn_thread(void *arg) {
}
}
// The inner disjunction over session_track_gtids handling must
// be grouped so it is only considered when special_sqlmode is
// false. Without the extra parentheses, operator precedence
// made the 'el.key() == "session_track_gtids"' branch a
// standalone failure condition that fired even when
// special_sqlmode == true.
if (
(special_sqlmode == true && verified_special_sqlmode == false) ||
(special_sqlmode == false &&
(special_sqlmode == false && (
(el.key() != "session_track_gtids" && (!values_equiv(k.value(), el.value()) || !values_equiv(s.value(), el.value()))) ||
(el.key() == "session_track_gtids" && !check_session_track_gtids(el.value(), s.value(), k.value()))
)
))
) {
__sync_fetch_and_add(&g_failed, 1);
testPassed = false;

@ -310,14 +310,20 @@ void * my_conn_thread(void *arg) {
}
}
// The inner disjunction over session_track_gtids handling must
// be grouped so it is only considered when special_sqlmode is
// false. Without the extra parentheses, operator precedence
// treated the 'el.key() == "session_track_gtids"' branch as a
// standalone failure condition that fired even when
// special_sqlmode == true.
if (
(special_sqlmode == true && verified_special_sqlmode == false) ||
(k == mysql_vars.end()) ||
(s == proxysql_vars["conn"].end()) ||
(special_sqlmode == false &&
(special_sqlmode == false && (
(el.key() != "session_track_gtids" && (!values_equiv(k.value(), el.value()) || !values_equiv(s.value(), el.value()))) ||
(el.key() == "session_track_gtids" && !check_session_track_gtids(el.value(), s.value(), k.value()))
)
))
) {
if (el.key() == "wsrep_sync_wait" && k == mysql_vars.end() && (s.value() == el.value())) {
variables_tested++;

Loading…
Cancel
Save