diff --git a/go/test/endtoend/vtgate/queries/aggregation/aggregation_test.go b/go/test/endtoend/vtgate/queries/aggregation/aggregation_test.go index db29644988e..13a5d628725 100644 --- a/go/test/endtoend/vtgate/queries/aggregation/aggregation_test.go +++ b/go/test/endtoend/vtgate/queries/aggregation/aggregation_test.go @@ -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) } @@ -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)]]`) +} diff --git a/go/test/endtoend/vtgate/queries/misc/misc_test.go b/go/test/endtoend/vtgate/queries/misc/misc_test.go index 2c8aece59fb..14448989956 100644 --- a/go/test/endtoend/vtgate/queries/misc/misc_test.go +++ b/go/test/endtoend/vtgate/queries/misc/misc_test.go @@ -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") +} diff --git a/go/vt/vtgate/engine/filter.go b/go/vt/vtgate/engine/filter.go index f36467a7526..fb696a9d679 100644 --- a/go/vt/vtgate/engine/filter.go +++ b/go/vt/vtgate/engine/filter.go @@ -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) } } diff --git a/go/vt/vtgate/evalengine/eval_result.go b/go/vt/vtgate/evalengine/eval_result.go index 2f2de033fdc..1971fdc39c2 100644 --- a/go/vt/vtgate/evalengine/eval_result.go +++ b/go/vt/vtgate/evalengine/eval_result.go @@ -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 diff --git a/go/vt/vtgate/planbuilder/operators/joins.go b/go/vt/vtgate/planbuilder/operators/joins.go index a91f6b43ffc..2764ad7f735 100644 --- a/go/vt/vtgate/planbuilder/operators/joins.go +++ b/go/vt/vtgate/planbuilder/operators/joins.go @@ -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() } diff --git a/go/vt/vtgate/planbuilder/testdata/from_cases.json b/go/vt/vtgate/planbuilder/testdata/from_cases.json index 8315a60e013..5687c950bdc 100644 --- a/go/vt/vtgate/planbuilder/testdata/from_cases.json +++ b/go/vt/vtgate/planbuilder/testdata/from_cases.json @@ -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" + ] + } } ]