Merge pull request #567 from erizocosmico/fix/reorder-unresolved

analyzer: wait until NaturalJoins are resolved on reorder rule

Author: ajnavarro

sql/analyzer/analyzer_test.go

@@ -353,3 +353,81 @@ func TestMixInnerAndNaturalJoins(t *testing.T) {
 	require.NoError(err)
 	require.Equal(expected, result)
 }
+
+func TestReorderProjectionUnresolvedChild(t *testing.T) {
+	require := require.New(t)
+	node := plan.NewProject(
+		[]sql.Expression{
+			expression.NewUnresolvedQualifiedColumn("rc", "commit_hash"),
+			expression.NewUnresolvedColumn("commit_author_when"),
+		},
+		plan.NewFilter(
+			expression.JoinAnd(
+				expression.NewEquals(
+					expression.NewUnresolvedQualifiedColumn("rc", "repository_id"),
+					expression.NewLiteral("foo", sql.Text),
+				),
+				expression.NewEquals(
+					expression.NewUnresolvedQualifiedColumn("rc", "ref_name"),
+					expression.NewLiteral("HEAD", sql.Text),
+				),
+				expression.NewEquals(
+					expression.NewUnresolvedQualifiedColumn("rc", "history_index"),
+					expression.NewLiteral(int64(0), sql.Int64),
+				),
+			),
+			plan.NewNaturalJoin(
+				plan.NewInnerJoin(
+					plan.NewUnresolvedTable("refs", ""),
+					plan.NewTableAlias("rc",
+						plan.NewUnresolvedTable("ref_commits", ""),
+					),
+					expression.NewAnd(
+						expression.NewEquals(
+							expression.NewUnresolvedQualifiedColumn("refs", "ref_name"),
+							expression.NewUnresolvedQualifiedColumn("rc", "ref_name"),
+						),
+						expression.NewEquals(
+							expression.NewUnresolvedQualifiedColumn("refs", "repository_id"),
+							expression.NewUnresolvedQualifiedColumn("rc", "repository_id"),
+						),
+					),
+				),
+				plan.NewTableAlias("c",
+					plan.NewUnresolvedTable("commits", ""),
+				),
+			),
+		),
+	)
+
+	commits := mem.NewTable("commits", sql.Schema{
+		{Name: "repository_id", Source: "commits", Type: sql.Text},
+		{Name: "commit_hash", Source: "commits", Type: sql.Text},
+		{Name: "commit_author_when", Source: "commits", Type: sql.Text},
+	})
+
+	refs := mem.NewTable("refs", sql.Schema{
+		{Name: "repository_id", Source: "refs", Type: sql.Text},
+		{Name: "ref_name", Source: "refs", Type: sql.Text},
+	})
+
+	refCommits := mem.NewTable("ref_commits", sql.Schema{
+		{Name: "repository_id", Source: "ref_commits", Type: sql.Text},
+		{Name: "ref_name", Source: "ref_commits", Type: sql.Text},
+		{Name: "commit_hash", Source: "ref_commits", Type: sql.Text},
+		{Name: "history_index", Source: "ref_commits", Type: sql.Int64},
+	})
+
+	db := mem.NewDatabase("")
+	db.AddTable("refs", refs)
+	db.AddTable("ref_commits", refCommits)
+	db.AddTable("commits", commits)
+
+	catalog := sql.NewCatalog()
+	catalog.AddDatabase(db)
+	a := withoutProcessTracking(NewDefault(catalog))
+
+	result, err := a.Analyze(sql.NewEmptyContext(), node)
+	require.NoError(err)
+	require.True(result.Resolved())
+}

sql/analyzer/optimization_rules.go

@@ -67,7 +67,16 @@ func reorderProjection(ctx *sql.Context, a *Analyzer, n sql.Node) (sql.Node, err
 	// Then we transform the projection
 	return n.TransformUp(func(node sql.Node) (sql.Node, error) {
 		project, ok := node.(*plan.Project)
-		if !ok {
+		// When we transform the projection, the children will always be
+		// unresolved in the case we want to fix, as the reorder happens just
+		// so some columns can be resolved.
+		// For that, we need to account for NaturalJoin, whose schema can't be
+		// obtained until it's resolved and ignore the projection for the
+		// moment until the resolve_natural_joins has finished resolving the
+		// node and we can tackle it in the next iteration.
+		// Without this check, it would cause a panic, because NaturalJoin's
+		// schema method is just a placeholder that should not be called.
+		if !ok || hasNaturalJoin(project.Child) {
 			return node, nil
 		}
 
@@ -420,3 +429,17 @@ func isTrue(e sql.Expression) bool {
 		lit.Type() == sql.Boolean &&
 		lit.Value().(bool)
 }
+
+// hasNaturalJoin checks whether there is a natural join at some point in the
+// given node and its children.
+func hasNaturalJoin(node sql.Node) bool {
+	var found bool
+	plan.Inspect(node, func(node sql.Node) bool {
+		if _, ok := node.(*plan.NaturalJoin); ok {
+			found = true
+			return false
+		}
+		return true
+	})
+	return found
+}