From 3c192cb11b5bd770de0e5bb6056fdb248fe6023d Mon Sep 17 00:00:00 2001
From: Harshit Gangal <harshit@planetscale.com>
Date: Fri, 30 Apr 2021 13:15:17 +0530
Subject: [PATCH] ignore the error and log as warn if not able to validate the
 current system settings value

Signed-off-by: Harshit Gangal <harshit@planetscale.com>
---
 go/vt/vtgate/engine/set.go      |  6 +++++-
 go/vt/vtgate/engine/set_test.go | 26 ++++++++++++++++++++++++--
 2 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/go/vt/vtgate/engine/set.go b/go/vt/vtgate/engine/set.go
index 43401cb8a25..dc317e52a23 100644
--- a/go/vt/vtgate/engine/set.go
+++ b/go/vt/vtgate/engine/set.go
@@ -247,7 +247,11 @@ func (svci *SysVarCheckAndIgnore) Execute(vcursor VCursor, env evalengine.Expres
 	checkSysVarQuery := fmt.Sprintf("select 1 from dual where @@%s = %s", svci.Name, svci.Expr)
 	result, err := execShard(vcursor, checkSysVarQuery, env.BindVars, rss[0], false /* rollbackOnError */, false /* canAutocommit */)
 	if err != nil {
-		return err
+		// Rather than returning the error, we will just log the error
+		// as the intention for executing the query it to validate the current setting and eventually ignore it anyways.
+		// There is no benefit of returning the error back to client.
+		log.Warningf("unable to validate the current settings for '%s': %s", svci.Name, err.Error())
+		return nil
 	}
 	if result.RowsAffected == 0 {
 		log.Infof("Ignored inapplicable SET %v = %v", svci.Name, svci.Expr)
diff --git a/go/vt/vtgate/engine/set_test.go b/go/vt/vtgate/engine/set_test.go
index ce2c0bae504..e6a3b0f45ad 100644
--- a/go/vt/vtgate/engine/set_test.go
+++ b/go/vt/vtgate/engine/set_test.go
@@ -17,6 +17,7 @@ limitations under the License.
 package engine
 
 import (
+	"errors"
 	"fmt"
 	"testing"
 
@@ -74,6 +75,7 @@ func TestSetTable(t *testing.T) {
 		expectedWarning  []*querypb.QueryWarning
 		expectedError    string
 		input            Primitive
+		execErr          error
 	}
 
 	tests := []testCase{
@@ -193,6 +195,25 @@ func TestSetTable(t *testing.T) {
 			},
 			expectedError: "Unexpected error, DestinationKeyspaceID mapping to multiple shards: DestinationAllShards()",
 		},
+		{
+			testName: "sysvar checkAndIgnore execute error",
+			setOps: []SetOp{
+				&SysVarCheckAndIgnore{
+					Name: "x",
+					Keyspace: &vindexes.Keyspace{
+						Name:    "ks",
+						Sharded: true,
+					},
+					TargetDestination: key.DestinationAnyShard{},
+					Expr:              "dummy_expr",
+				},
+			},
+			expectedQueryLog: []string{
+				`ResolveDestinations ks [] Destinations:DestinationAnyShard()`,
+				`ExecuteMultiShard ks.-20: select 1 from dual where @@x = dummy_expr {} false false`,
+			},
+			execErr: errors.New("some random error"),
+		},
 		{
 			testName: "udv ignore checkAndIgnore ",
 			setOps: []SetOp{
@@ -299,8 +320,9 @@ func TestSetTable(t *testing.T) {
 				Input: tc.input,
 			}
 			vc := &loggingVCursor{
-				shards:  []string{"-20", "20-"},
-				results: tc.qr,
+				shards:         []string{"-20", "20-"},
+				results:        tc.qr,
+				multiShardErrs: []error{tc.execErr},
 			}
 			_, err := set.Execute(vc, map[string]*querypb.BindVariable{}, false)
 			if tc.expectedError == "" {