From 43708d7b65d598801fb777d046ded1fc985b835e Mon Sep 17 00:00:00 2001
From: "Brian Fraser fraserb@gmail.com" <>
Date: Mon, 30 Jul 2012 11:44:28 -0300
Subject: [PATCH] VariableAdvisorRules: Check that log_bin is ON instead of
 doing log_bin ? 1 : 0

---
 bin/pt-variable-advisor      |  8 ++++----
 lib/VariableAdvisorRules.pm  |  8 ++++----
 t/lib/VariableAdvisorRules.t | 21 +++++++++++++++++++--
 3 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/bin/pt-variable-advisor b/bin/pt-variable-advisor
index 1a5eaf64..fb23d538 100755
--- a/bin/pt-variable-advisor
+++ b/bin/pt-variable-advisor
@@ -3202,7 +3202,7 @@ sub get_rules {
       code => sub {
          my ( %args ) = @_;
          return _var_eq($args{variables}->{expire_log_days}, 0)
-            && $args{variables}->{log_bin} ? 1 : 0;
+            && _var_seq($args{variables}->{log_bin}, "ON");
       },
    },
    {
@@ -3235,7 +3235,7 @@ sub get_rules {
       code => sub {
          my ( %args ) = @_;
          return _var_seq($args{variables}->{innodb_locks_unsafe_for_binlog},
-            "ON") && $args{variables}->{log_bin} ? 1 : 0;
+            "ON") && _var_seq($args{variables}->{log_bin}, "ON");
       },
    },
    {
@@ -3243,7 +3243,7 @@ sub get_rules {
       code => sub {
          my ( %args ) = @_;
          return _var_sneq($args{variables}->{innodb_support_xa}, "ON")
-            && $args{variables}->{log_bin} ? 1 : 0;
+            && _var_seq($args{variables}->{log_bin}, "ON");
       },
    },
    {
@@ -3291,7 +3291,7 @@ sub get_rules {
       code => sub {
          my ( %args ) = @_;
          return
-            $args{variables}->{log_bin}
+            _var_seq($args{variables}->{log_bin}, "ON")
             && (   _var_eq($args{variables}->{sync_binlog}, 0)
                 || _var_gt($args{variables}->{sync_binlog}, 1)) ? 1 : 0;
       },
diff --git a/lib/VariableAdvisorRules.pm b/lib/VariableAdvisorRules.pm
index 703ed7fd..16a1af05 100644
--- a/lib/VariableAdvisorRules.pm
+++ b/lib/VariableAdvisorRules.pm
@@ -431,7 +431,7 @@ sub get_rules {
       code => sub {
          my ( %args ) = @_;
          return _var_eq($args{variables}->{expire_log_days}, 0)
-            && $args{variables}->{log_bin} ? 1 : 0;
+            && _var_seq($args{variables}->{log_bin}, "ON");
       },
    },
    {
@@ -464,7 +464,7 @@ sub get_rules {
       code => sub {
          my ( %args ) = @_;
          return _var_seq($args{variables}->{innodb_locks_unsafe_for_binlog},
-            "ON") && $args{variables}->{log_bin} ? 1 : 0;
+            "ON") && _var_seq($args{variables}->{log_bin}, "ON");
       },
    },
    {
@@ -472,7 +472,7 @@ sub get_rules {
       code => sub {
          my ( %args ) = @_;
          return _var_sneq($args{variables}->{innodb_support_xa}, "ON")
-            && $args{variables}->{log_bin} ? 1 : 0;
+            && _var_seq($args{variables}->{log_bin}, "ON");
       },
    },
    {
@@ -520,7 +520,7 @@ sub get_rules {
       code => sub {
          my ( %args ) = @_;
          return
-            $args{variables}->{log_bin}
+            _var_seq($args{variables}->{log_bin}, "ON")
             && (   _var_eq($args{variables}->{sync_binlog}, 0)
                 || _var_gt($args{variables}->{sync_binlog}, 1)) ? 1 : 0;
       },
diff --git a/t/lib/VariableAdvisorRules.t b/t/lib/VariableAdvisorRules.t
index c21881b7..1fce95da 100644
--- a/t/lib/VariableAdvisorRules.t
+++ b/t/lib/VariableAdvisorRules.t
@@ -9,7 +9,7 @@ BEGIN {
 use strict;
 use warnings FATAL => 'all';
 use English qw(-no_match_vars);
-use Test::More tests => 83;
+use Test::More;
 
 use PodParser;
 use AdvisorRules;
@@ -279,6 +279,10 @@ my @cases = (
       vars   => [qw(expire_log_days 0 log_bin ON)],
       advice => [qw(expire_log_days)],
    },
+   {  name   => "expire_log_days, log_bin OFF, only warns about log_bin",
+      vars   => [qw(expire_log_days 0 log_bin OFF)],
+      advice => [qw(log_bin)],
+   },
    {  name   => "innodb_file_io_threads",
       vars   => [qw(innodb_file_io_threads 16)],
       advice => [qw(innodb_file_io_threads)],
@@ -295,10 +299,18 @@ my @cases = (
       vars   => [qw(innodb_locks_unsafe_for_binlog ON log_bin ON)],
       advice => [qw(innodb_locks_unsafe_for_binlog)],
    },
+   {  name   => "innodb_locks_unsafe_for_binlog, log_bin off, only warns about log_bin",
+      vars   => [qw(innodb_locks_unsafe_for_binlog ON log_bin OFF)],
+      advice => [qw(log_bin)],
+   },
    {  name   => "innodb_support_xa",
       vars   => [qw(innodb_support_xa OFF log_bin ON)],
       advice => [qw(innodb_support_xa)],
    },
+   {  name   => "innodb_support_xa, log_bin OFF, only warns about log_bin",
+      vars   => [qw(innodb_support_xa OFF log_bin OFF)],
+      advice => [qw(log_bin)],
+   },
    {  name   => "log_bin ON",
       vars   => [qw(log_bin ON)],
       advice => [qw()],
@@ -339,6 +351,10 @@ my @cases = (
       vars   => [qw(sync_binlog 2 log_bin ON)],
       advice => [qw(sync_binlog)],
    },
+   {  name   => "log_bin OFF, sync_binlog 0, doesn't warn about sync_binlog",
+      vars   => [qw(sync_binlog 0 log_bin OFF)],
+      advice => [qw(log_bin)],
+   },
    {  name   => "tmp_table_size",
       vars   => [qw(tmp_table_size 1024 max_heap_table_size 512)],
       advice => [qw(tmp_table_size)],
@@ -404,4 +420,5 @@ like(
    qr/Complete test coverage/,
    '_d() works'
 );
-exit;
+
+done_testing;