addressed review comments

Signed-off-by: Andres Taylor <andres@planetscale.com>
vitessio · Oct 11, 2021 · ba23ffa · ba23ffa
1 parent 25767ce
commit ba23ffa
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 33 deletions.
diff --git a/go/vt/vtgate/planbuilder/abstract/queryprojection.go b/go/vt/vtgate/planbuilder/abstract/queryprojection.go
@@ -260,34 +260,40 @@ func (qp *QueryProjection) getSimplifiedExpr(e sqlparser.Expr, semTable *semanti
 	// Eg - select music.foo as bar, weightstring(music.foo) from music order by bar
 
 	colExpr, isColName := e.(*sqlparser.ColName)
-	if isColName {
-		tblInfo, _ := semTable.TableInfoForExpr(e)
-		if tblInfo != nil {
-			if dTablInfo, ok := tblInfo.(*semantics.DerivedTable); ok {
-				weightStrExpr, err = semantics.RewriteDerivedExpression(colExpr, dTablInfo)
-				if err != nil {
-					return nil, nil, err
-				}
-				return e, weightStrExpr, nil
-			}
-		}
+	if !isColName {
+		return e, e, nil
+	}
+
+	if sqlparser.IsNull(e) {
+		return e, nil, nil
+	}
 
-		if colExpr.Qualifier.IsEmpty() {
-			for _, selectExpr := range qp.SelectExprs {
-				aliasedExpr, isAliasedExpr := selectExpr.Col.(*sqlparser.AliasedExpr)
-				if !isAliasedExpr {
-					continue
-				}
-				isAliasExpr := !aliasedExpr.As.IsEmpty()
-				if isAliasExpr && colExpr.Name.Equal(aliasedExpr.As) {
-					return e, aliasedExpr.Expr, nil
-				}
+	tblInfo, err := semTable.TableInfoForExpr(e)
+	if err != nil && err != semantics.ErrMultipleTables {
+		// we can live with ErrMultipleTables and just ignore it. anything else should fail this method
+		return nil, nil, err
+	}
+	if tblInfo != nil {
+		if dTablInfo, ok := tblInfo.(*semantics.DerivedTable); ok {
+			weightStrExpr, err = semantics.RewriteDerivedExpression(colExpr, dTablInfo)
+			if err != nil {
+				return nil, nil, err
 			}
+			return e, weightStrExpr, nil
 		}
 	}
 
-	if sqlparser.IsNull(e) {
-		return e, nil, nil
+	if colExpr.Qualifier.IsEmpty() {
+		for _, selectExpr := range qp.SelectExprs {
+			aliasedExpr, isAliasedExpr := selectExpr.Col.(*sqlparser.AliasedExpr)
+			if !isAliasedExpr {
+				continue
+			}
+			isAliasExpr := !aliasedExpr.As.IsEmpty()
+			if isAliasExpr && colExpr.Name.Equal(aliasedExpr.As) {
+				return e, aliasedExpr.Expr, nil
+			}
+		}
 	}
 
 	return e, e, nil

diff --git a/go/vt/vtgate/planbuilder/horizon_planning.go b/go/vt/vtgate/planbuilder/horizon_planning.go
@@ -274,10 +274,9 @@ func pushProjection(expr *sqlparser.AliasedExpr, plan logicalPlan, semTable *sem
 }
 
 func rewriteProjectionOfDerivedTable(expr *sqlparser.AliasedExpr, semTable *semantics.SemTable) error {
-	var err error
-	ti, _ := semTable.TableInfoForExpr(expr.Expr)
-	if ti == nil {
-		return nil
+	ti, err := semTable.TableInfoForExpr(expr.Expr)
+	if err != nil && err != semantics.ErrMultipleTables {
+		return err
 	}
 	_, isDerivedTable := ti.(*semantics.DerivedTable)
 	if isDerivedTable {

diff --git a/go/vt/vtgate/semantics/derived_table.go b/go/vt/vtgate/semantics/derived_table.go
@@ -78,10 +78,7 @@ func (dt *DerivedTable) dependencies(colName string, org originable) (dependenci
 		return &nothing{}, nil
 	}
 
-	recursive := dt.tables
-	direct := org.tableSetFor(dt.ASTNode)
-
-	return createUncertain(direct, recursive), nil
+	return createUncertain(directDeps, dt.tables), nil
 }
 
 // IsInfSchema implements the TableInfo interface

diff --git a/go/vt/vtgate/semantics/scoper.go b/go/vt/vtgate/semantics/scoper.go
@@ -83,8 +83,8 @@ func (s *scoper) down(cursor *sqlparser.Cursor) error {
 			break
 		}
 
-		// adding a VTableInfo for each SELECT, so it can be used by GROUP BY, HAVING, ORDER BY
-		// the VTableInfo we are creating here should not be confused with derived tables' VTableInfo
+		// adding a vTableInfo for each SELECT, so it can be used by GROUP BY, HAVING, ORDER BY
+		// the vTableInfo we are creating here should not be confused with derived tables' vTableInfo
 		wScope, exists := s.wScope[sel]
 		if !exists {
 			break