Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[16.0] bugfixes: collection of fixes to bugs found while fuzzing #13805

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ func start(t *testing.T) (utils.MySQLCompare, func()) {
deleteAll := func() {
_, _ = utils.ExecAllowError(t, mcmp.VtConn, "set workload = oltp")

tables := []string{"t9", "aggr_test", "t3", "t7_xxhash", "aggr_test_dates", "t7_xxhash_idx", "t1", "t2"}
tables := []string{"t9", "aggr_test", "t3", "t7_xxhash", "aggr_test_dates", "t7_xxhash_idx", "t1", "t2", "t10"}
for _, table := range tables {
_, _ = mcmp.ExecAndIgnore("delete from " + table)
}
Expand Down Expand Up @@ -438,3 +438,15 @@ func TestAggregationRandomOnAnAggregatedValue(t *testing.T) {
mcmp.AssertMatchesNoOrder("select /*vt+ PLANNER=gen4 */ A.a, A.b, (A.a / A.b) as d from (select sum(a) as a, sum(b) as b from t10 where a = 100) A;",
`[[DECIMAL(100) DECIMAL(10) DECIMAL(10.0000)]]`)
}

func TestBuggyQueries(t *testing.T) {
// These queries have been found to be producing the wrong results by the query fuzzer
// Adding them as end2end tests to make sure we never get them wrong again
mcmp, closer := start(t)
defer closer()

mcmp.Exec("insert into t10(k, a, b) values (0, 100, 10), (10, 200, 20), (20, null, null)")

mcmp.AssertMatches("select /*vt+ PLANNER=Gen4 */ sum(t1.a) from t10 as t1, t10 as t2",
`[[DECIMAL(900)]]`)
}
10 changes: 10 additions & 0 deletions go/test/endtoend/vtgate/queries/misc/misc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,3 +230,13 @@ func TestHighNumberOfParams(t *testing.T) {
}
require.Equal(t, 5, count)
}

func TestBuggyOuterJoin(t *testing.T) {
// We found a couple of inconsistencies around outer joins, adding these tests to stop regressions
mcmp, closer := start(t)
defer closer()

mcmp.Exec("insert into t1(id1, id2) values (1,2), (42,5), (5, 42)")

mcmp.Exec("select t1.id1, t2.id1 from t1 left join t1 as t2 on t2.id1 = t2.id2")
}
7 changes: 2 additions & 5 deletions go/vt/vtgate/engine/filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,8 @@ func (f *Filter) TryExecute(ctx context.Context, vcursor VCursor, bindVars map[s
if err != nil {
return nil, err
}
intEvalResult, err := evalResult.Value().ToInt64()
if err != nil {
return nil, err
}
if intEvalResult == 1 {

if evalResult.ToBoolean() {
rows = append(rows, row)
}
}
Expand Down
4 changes: 4 additions & 0 deletions go/vt/vtgate/evalengine/eval_result.go
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,10 @@ func (er *EvalResult) isTextual() bool {
return sqltypes.IsText(tt) || sqltypes.IsBinary(tt)
}

func (er *EvalResult) ToBoolean() bool {
return er.isTruthy() == boolTrue
}

func (er *EvalResult) isTruthy() boolean {
if er.isNull() {
return boolNULL
Expand Down
2 changes: 1 addition & 1 deletion go/vt/vtgate/planbuilder/operators/joins.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func AddPredicate(join JoinOp, ctx *plancontext.PlanningContext, expr sqlparser.
case deps.IsSolvedBy(TableID(join.GetRHS())):
// if we are dealing with an outer join, always start by checking if this predicate can turn
// the join into an inner join
if !join.IsInner() && canConvertToInner(ctx, expr, TableID(join.GetRHS())) {
if !joinPredicates && !join.IsInner() && canConvertToInner(ctx, expr, TableID(join.GetRHS())) {
join.MakeInner()
}

Expand Down
78 changes: 78 additions & 0 deletions go/vt/vtgate/planbuilder/testdata/from_cases.json
Original file line number Diff line number Diff line change
Expand Up @@ -6421,5 +6421,83 @@
"query": "select missing_column from unsharded, unsharded_tab",
"v3-plan": "VT03019: symbol missing_column not found",
"gen4-plan": "Column 'missing_column' in field list is ambiguous"
},
{
"comment": "join predicate only depending on the RHS should not turn outer join into inner join",
"query": "select t1.id1, t2.id1 from t1 left join t1 as t2 on t2.id1 = t2.id2",
"v3-plan": {
"QueryType": "SELECT",
"Original": "select t1.id1, t2.id1 from t1 left join t1 as t2 on t2.id1 = t2.id2",
"Instructions": {
"OperatorType": "Join",
"Variant": "LeftJoin",
"JoinColumnIndexes": "L:0,R:0",
"TableName": "t1_t1",
"Inputs": [
{
"OperatorType": "Route",
"Variant": "Scatter",
"Keyspace": {
"Name": "zlookup_unique",
"Sharded": true
},
"FieldQuery": "select t1.id1 from t1 where 1 != 1",
"Query": "select t1.id1 from t1",
"Table": "t1"
},
{
"OperatorType": "Route",
"Variant": "Scatter",
"Keyspace": {
"Name": "zlookup_unique",
"Sharded": true
},
"FieldQuery": "select t2.id1 from t1 as t2 where 1 != 1",
"Query": "select t2.id1 from t1 as t2 where t2.id1 = t2.id2",
"Table": "t1"
}
]
},
"TablesUsed": [
"zlookup_unique.t1"
]
},
"gen4-plan": {
"QueryType": "SELECT",
"Original": "select t1.id1, t2.id1 from t1 left join t1 as t2 on t2.id1 = t2.id2",
"Instructions": {
"OperatorType": "Join",
"Variant": "LeftJoin",
"JoinColumnIndexes": "L:0,R:0",
"TableName": "t1_t1",
"Inputs": [
{
"OperatorType": "Route",
"Variant": "Scatter",
"Keyspace": {
"Name": "zlookup_unique",
"Sharded": true
},
"FieldQuery": "select t1.id1 from t1 where 1 != 1",
"Query": "select t1.id1 from t1",
"Table": "t1"
},
{
"OperatorType": "Route",
"Variant": "Scatter",
"Keyspace": {
"Name": "zlookup_unique",
"Sharded": true
},
"FieldQuery": "select t2.id1 from t1 as t2 where 1 != 1",
"Query": "select t2.id1 from t1 as t2 where t2.id1 = t2.id2",
"Table": "t1"
}
]
},
"TablesUsed": [
"zlookup_unique.t1"
]
}
}
]