[release-16.0] planner fix: scoping rules for JOIN ON expression insi…

…de a subquery (#12891) * planner fix: scoping rules for JOIN ON expression inside a subquery (#12881) * fix: scoping rules for on clause expression inside a subquery Signed-off-by: Harshit Gangal <[email protected]> * find the parent scope of the statement in on clause Signed-off-by: harshit-gangal <[email protected]> * addressed review comments Signed-off-by: harshit-gangal <[email protected]> --------- Signed-off-by: Harshit Gangal <[email protected]> Signed-off-by: harshit-gangal <[email protected]> Signed-off-by: Florent Poinsard <[email protected]> * Fix plan test expectation for v3 Signed-off-by: Florent Poinsard <[email protected]> --------- Signed-off-by: Harshit Gangal <[email protected]> Signed-off-by: harshit-gangal <[email protected]> Signed-off-by: Florent Poinsard <[email protected]> Co-authored-by: Harshit Gangal <[email protected]>
vitessio · Apr 14, 2023 · e18974d · e18974d
1 parent 9dcbd7d
commit e18974d
Show file tree

Hide file tree

Showing 6 changed files with 76 additions and 13 deletions.
diff --git a/go/vt/vtgate/planbuilder/testdata/select_cases.json b/go/vt/vtgate/planbuilder/testdata/select_cases.json
@@ -8041,5 +8041,32 @@
         "main.unsharded_a"
       ]
     }
+  },
+  {
+    "comment": "subquery having join table on clause, using column reference of outer select table",
+    "query": "select (select 1 from user u1 join user u2 on u1.id = u2.id and u1.id = u3.id) subquery from user u3 where u3.id = 1",
+    "v3-plan": "VT03019: symbol u3.id not found",
+    "gen4-plan": {
+      "QueryType": "SELECT",
+      "Original": "select (select 1 from user u1 join user u2 on u1.id = u2.id and u1.id = u3.id) subquery from user u3 where u3.id = 1",
+      "Instructions": {
+        "OperatorType": "Route",
+        "Variant": "EqualUnique",
+        "Keyspace": {
+          "Name": "user",
+          "Sharded": true
+        },
+        "FieldQuery": "select (select 1 from `user` as u1 join `user` as u2 on u1.id = u2.id and u1.id = u3.id where 1 != 1) as subquery from `user` as u3 where 1 != 1",
+        "Query": "select (select 1 from `user` as u1 join `user` as u2 on u1.id = u2.id and u1.id = u3.id) as subquery from `user` as u3 where u3.id = 1",
+        "Table": "`user`",
+        "Values": [
+          "INT64(1)"
+        ],
+        "Vindex": "user_index"
+      },
+      "TablesUsed": [
+        "user.user"
+      ]
+    }
   }
 ]
diff --git a/go/vt/vtgate/semantics/analyzer_test.go b/go/vt/vtgate/semantics/analyzer_test.go
@@ -320,7 +320,7 @@ func TestMissingTable(t *testing.T) {
 
 func TestUnknownColumnMap2(t *testing.T) {
 	varchar := querypb.Type_VARCHAR
-	int := querypb.Type_INT32
+	integer := querypb.Type_INT32
 
 	authoritativeTblA := vindexes.Table{
 		Name: sqlparser.NewIdentifierCS("a"),
@@ -346,15 +346,15 @@ func TestUnknownColumnMap2(t *testing.T) {
 		Name: sqlparser.NewIdentifierCS("a"),
 		Columns: []vindexes.Column{{
 			Name: sqlparser.NewIdentifierCI("col"),
-			Type: int,
+			Type: integer,
 		}},
 		ColumnListAuthoritative: true,
 	}
 	authoritativeTblBWithInt := vindexes.Table{
 		Name: sqlparser.NewIdentifierCS("b"),
 		Columns: []vindexes.Column{{
 			Name: sqlparser.NewIdentifierCI("col"),
-			Type: int,
+			Type: integer,
 		}},
 		ColumnListAuthoritative: true,
 	}
@@ -391,7 +391,7 @@ func TestUnknownColumnMap2(t *testing.T) {
 		name:   "authoritative columns",
 		schema: map[string]*vindexes.Table{"a": &authoritativeTblA, "b": &authoritativeTblBWithInt},
 		err:    false,
-		typ:    &int,
+		typ:    &integer,
 	}, {
 		name:   "authoritative columns with overlap",
 		schema: map[string]*vindexes.Table{"a": &authoritativeTblAWithConflict, "b": &authoritativeTblB},
@@ -1434,6 +1434,28 @@ func TestSingleUnshardedKeyspace(t *testing.T) {
 	}
 }
 
+// TestScopingSubQueryJoinClause tests the scoping behavior of a subquery containing a join clause.
+// The test ensures that the scoping analysis correctly identifies and handles the relationships
+// between the tables involved in the join operation with the outer query.
+func TestScopingSubQueryJoinClause(t *testing.T) {
+	query := "select (select 1 from u1 join u2 on u1.id = u2.id and u2.id = u3.id) x from u3"
+
+	parse, err := sqlparser.Parse(query)
+	require.NoError(t, err)
+
+	st, err := Analyze(parse, "user", &FakeSI{
+		Tables: map[string]*vindexes.Table{
+			"t": {Name: sqlparser.NewIdentifierCS("t")},
+		},
+	})
+	require.NoError(t, err)
+	require.NoError(t, st.NotUnshardedErr)
+
+	tb := st.DirectDeps(parse.(*sqlparser.Select).SelectExprs[0].(*sqlparser.AliasedExpr).Expr.(*sqlparser.Subquery).Select.(*sqlparser.Select).From[0].(*sqlparser.JoinTableExpr).Condition.On)
+	require.Equal(t, 3, tb.NumberOfTables())
+
+}
+
 var ks1 = &vindexes.Keyspace{
 	Name:    "ks1",
 	Sharded: false,

diff --git a/go/vt/vtgate/semantics/early_rewriter_test.go b/go/vt/vtgate/semantics/early_rewriter_test.go
@@ -180,6 +180,9 @@ func TestExpandStar(t *testing.T) {
 			require.True(t, isSelectStatement, "analyzer expects a select statement")
 			st, err := Analyze(selectStatement, cDB, schemaInfo)
 			if tcase.expErr == "" {
+				require.NoError(t, err)
+				require.NoError(t, st.NotUnshardedErr)
+				require.NoError(t, st.NotSingleRouteErr)
 				found := 0
 			outer:
 				for _, selExpr := range selectStatement.SelectExprs {
@@ -204,9 +207,6 @@ func TestExpandStar(t *testing.T) {
 				} else {
 					require.Equal(t, tcase.colExpandedNumber, found)
 				}
-				require.NoError(t, err)
-				require.NoError(t, st.NotUnshardedErr)
-				require.NoError(t, st.NotSingleRouteErr)
 				assert.Equal(t, tcase.expSQL, sqlparser.String(selectStatement))
 			} else {
 				require.EqualError(t, err, tcase.expErr)

diff --git a/go/vt/vtgate/semantics/scoper.go b/go/vt/vtgate/semantics/scoper.go
@@ -46,6 +46,7 @@ type (
 		tables    []TableInfo
 		isUnion   bool
 		joinUsing map[string]TableSet
+		stmtScope bool
 	}
 )
 
@@ -62,11 +63,13 @@ func (s *scoper) down(cursor *sqlparser.Cursor) error {
 	switch node := node.(type) {
 	case *sqlparser.Update, *sqlparser.Delete:
 		currScope := newScope(s.currentScope())
+		currScope.stmtScope = true
 		s.push(currScope)
 
 		currScope.stmt = node.(sqlparser.Statement)
 	case *sqlparser.Select:
 		currScope := newScope(s.currentScope())
+		currScope.stmtScope = true
 		s.push(currScope)
 
 		// Needed for order by with Literal to find the Expression.
@@ -77,10 +80,10 @@ func (s *scoper) down(cursor *sqlparser.Cursor) error {
 	case sqlparser.TableExpr:
 		if isParentSelect(cursor) {
 			// when checking the expressions used in JOIN conditions, special rules apply where the ON expression
-			// can only see the two tables involved in the JOIN, and no other tables.
-			// To create this special context, we create a special scope here that is then merged with
-			// the surrounding scope when we come back out from the JOIN
-			nScope := newScope(nil)
+			// can only see the two tables involved in the JOIN, and no other tables of that select statement.
+			// They are allowed to see the tables of the outer select query.
+			// To create this special context, we will find the parent scope of the select statement involved.
+			nScope := newScope(s.currentScope().findParentScopeOfStatement())
 			nScope.stmt = cursor.Parent().(*sqlparser.Select)
 			s.push(nScope)
 		}
@@ -289,3 +292,14 @@ func (s *scope) prepareUsingMap() (result map[TableSet]map[string]TableSet) {
 	}
 	return
 }
+
+// findParentScopeOfStatement finds the scope that belongs to a statement.
+func (s *scope) findParentScopeOfStatement() *scope {
+	if s.stmtScope {
+		return s.parent
+	}
+	if s.parent == nil {
+		return nil
+	}
+	return s.parent.findParentScopeOfStatement()
+}
diff --git a/go/vt/vtgate/semantics/semantic_state.go b/go/vt/vtgate/semantics/semantic_state.go
@@ -364,7 +364,7 @@ var _ evalengine.TranslationLookup = (*SemTable)(nil)
 var columnNotSupportedErr = vterrors.Errorf(vtrpcpb.Code_UNIMPLEMENTED, "column access not supported here")
 
 // ColumnLookup implements the TranslationLookup interface
-func (st *SemTable) ColumnLookup(col *sqlparser.ColName) (int, error) {
+func (st *SemTable) ColumnLookup(*sqlparser.ColName) (int, error) {
 	return 0, columnNotSupportedErr
 }
 

diff --git a/go/vt/vtgate/semantics/table_set.go b/go/vt/vtgate/semantics/table_set.go
@@ -27,7 +27,7 @@ import (
 type TableSet bitset.Bitset
 
 // Format formats the TableSet.
-func (ts TableSet) Format(f fmt.State, verb rune) {
+func (ts TableSet) Format(f fmt.State, _ rune) {
 	first := true
 	fmt.Fprintf(f, "TableSet{")
 	bitset.Bitset(ts).ForEach(func(tid int) {