Skip to content

Commit

Permalink
Merge pull request #8004 from planetscale/sysvar-ci
Browse files Browse the repository at this point in the history
Ignore the error and log as warn if not able to validate the current system setting for check and ignore case
  • Loading branch information
systay authored May 3, 2021
2 parents fb1336a + f70c410 commit 991fe87
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 991fe87

Please sign in to comment.