Repository: vitessio/vitess
Pull request #8004 (2021-04-30 07:47:22)

Ignore the error and log as warn if not able to validate the current system setting for check and ignore case
…system settings value

Signed-off-by: Harshit Gangal <harshit@planetscale.com>

<!--
  Thank you for your contribution to the Vitess project.
  How to contribute: https://vitess.io/docs/contributing/
  Please first make sure there is an open Issue to discuss the feature/fix suggested in this PR.
  If this is a new feature, please mark the Issue as "RFC".
 -->

<!-- if this PR is Work in Progress please create it as a Draft Pull Request -->

## Description
<!-- A few sentences describing the overall goals of the pull request's commits. -->
<!-- If this is a bug fix and you think the fix should be backported, please write so. -->

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.
	

## Related Issue(s)
<!-- List related issues and pull requests. If this PR fixes an issue, please add it using Fixes #????  -->

Fixes https://github.com/vitessio/vitess/issues/7981

## Checklist
- [X] Tests were added or are not required
- [X] Documentation was added or is not required



Patch:
diff --git a/go/vt/vtgate/engine/set.go b/go/vt/vtgate/engine/set.go
index 2b6d9f0c1d..f15c506c50 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 len(result.Rows) == 0 {
 		log.Infof("Ignored inapplicable SET %v = %v", svci.Name, svci.Expr)


Test patch:
diff --git a/go/vt/vtgate/engine/set_test.go b/go/vt/vtgate/engine/set_test.go
index ce2c0bae50..e6a3b0f45a 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 == "" {