Skip to content

Commit

Permalink
ignore the error and log as warn if not able to validate the current …
Browse files Browse the repository at this point in the history
…system settings value

Signed-off-by: Harshit Gangal <harshit@planetscale.com>
  • Loading branch information
harshit-gangal committed Apr 30, 2021
1 parent 704f98f commit f70c410
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 3 deletions.
6 changes: 5 additions & 1 deletion go/vt/vtgate/engine/set.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
26 changes: 24 additions & 2 deletions go/vt/vtgate/engine/set_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package engine

import (
"errors"
"fmt"
"testing"

Expand Down Expand Up @@ -74,6 +75,7 @@ func TestSetTable(t *testing.T) {
expectedWarning []*querypb.QueryWarning
expectedError string
input Primitive
execErr error
}

tests := []testCase{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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 == "" {
Expand Down

0 comments on commit f70c410

Please sign in to comment.