opt: use Presentation for cteSource.cols in optbuilder

The columns in `cteSource` are only used for their IDs and names,
which is better represented by a `Presentation`. This will make things
easier for recursive CTEs.

We also make the "no columns" error more general since our syntax
allows more types of statements inside WITH.

Release note: None

pkg/sql/explain_test.go

@@ -159,7 +159,7 @@ func TestStatementReuses(t *testing.T) {
 			t.Run(test, func(t *testing.T) {
 				rows, err := db.Query("EXPLAIN WITH a AS (" + test + ") TABLE a")
 				if err != nil {
-					if testutils.IsError(err, "does not have a RETURNING clause") {
+					if testutils.IsError(err, "does not return any columns") {
 						// This error is acceptable and does not constitute a test failure.
 						return
 					}

pkg/sql/logictest/testdata/logic_test/with

@@ -133,7 +133,7 @@ SELECT * from x
 ----
 
 # #22420: ensure okay error message for CTE clause without output columns
-query error WITH clause "t" does not have a RETURNING clause
+query error WITH clause "t" does not return any columns
 WITH t AS (
     INSERT INTO x(a) VALUES(0)
 )

pkg/sql/opt/optbuilder/scope.go

@@ -95,7 +95,7 @@ type scope struct {
 // cteSource represents a CTE in the given query.
 type cteSource struct {
 	name         tree.AliasClause
-	cols         []scopeColumn
+	cols         physical.Presentation
 	originalExpr tree.Statement
 	expr         memo.RelExpr
 	id           opt.WithID
@@ -240,25 +240,30 @@ func (s *scope) makeOrderingChoice() physical.OrderingChoice {
 // makePhysicalProps constructs physical properties using the columns in the
 // scope for presentation and s.ordering for required ordering.
 func (s *scope) makePhysicalProps() *physical.Required {
-	p := &physical.Required{}
-
-	if len(s.cols) > 0 {
-		p.Presentation = make(physical.Presentation, 0, len(s.cols))
-		for i := range s.cols {
-			col := &s.cols[i]
-			if !col.hidden {
-				p.Presentation = append(p.Presentation, opt.AliasedColumn{
-					Alias: string(col.name),
-					ID:    col.id,
-				})
-			}
-		}
+	p := &physical.Required{
+		Presentation: s.makePresentation(),
 	}
-
 	p.Ordering.FromOrdering(s.ordering)
 	return p
 }
 
+func (s *scope) makePresentation() physical.Presentation {
+	if len(s.cols) == 0 {
+		return nil
+	}
+	presentation := make(physical.Presentation, 0, len(s.cols))
+	for i := range s.cols {
+		col := &s.cols[i]
+		if !col.hidden {
+			presentation = append(presentation, opt.AliasedColumn{
+				Alias: string(col.name),
+				ID:    col.id,
+			})
+		}
+	}
+	return presentation
+}
+
 // walkExprTree walks the given expression and performs name resolution,
 // replaces unresolved column names with columnProps, and replaces subqueries
 // with typed subquery structs.

pkg/sql/opt/optbuilder/select.go

@@ -78,11 +78,10 @@ func (b *Builder) buildDataSource(
 			outScope.cols = nil
 			i := 0
 			for _, col := range cte.cols {
-				id := col.id
-				c := b.factory.Metadata().ColumnMeta(id)
-				newCol := b.synthesizeColumn(outScope, string(col.name), c.Type, nil, nil)
+				c := b.factory.Metadata().ColumnMeta(col.ID)
+				newCol := b.synthesizeColumn(outScope, col.Alias, c.Type, nil, nil)
 				newCol.table = *tn
-				inCols[i] = id
+				inCols[i] = col.ID
 				outCols[i] = newCol.id
 				i++
 			}
@@ -582,8 +581,7 @@ func (b *Builder) buildCTE(
 	outScope.ctes = make(map[string]*cteSource)
 	for i := range ctes {
 		cteScope := b.buildStmt(ctes[i].Stmt, nil /* desiredTypes */, outScope)
-		cteScope.removeHiddenCols()
-		cols := cteScope.cols
+		cols := b.getCTECols(cteScope, ctes[i].Name)
 		name := ctes[i].Name.Alias
 
 		// TODO(justin): lift this restriction when possible. WITH should be hoistable.
@@ -598,30 +596,7 @@ func (b *Builder) buildCTE(
 			)
 		}
 
-		// Names for the output columns can optionally be specified.
-		if ctes[i].Name.Cols != nil {
-			if len(cteScope.cols) != len(ctes[i].Name.Cols) {
-				panic(pgerror.Newf(
-					pgcode.InvalidColumnReference,
-					"source %q has %d columns available but %d columns specified",
-					name, len(cteScope.cols), len(ctes[i].Name.Cols),
-				))
-			}
-
-			cols = make([]scopeColumn, len(cteScope.cols))
-			tableName := tree.MakeUnqualifiedTableName(ctes[i].Name.Alias)
-			copy(cols, cteScope.cols)
-			for j := range cols {
-				cols[j].name = ctes[i].Name.Cols[j]
-				cols[j].table = tableName
-			}
-		}
-
-		if len(cols) == 0 {
-			panic(pgerror.Newf(pgcode.FeatureNotSupported,
-				"WITH clause %q does not have a RETURNING clause", tree.ErrString(&name)))
-		}
-
+		cteScope.removeHiddenCols()
 		projectionsScope := cteScope.replace()
 		projectionsScope.appendColumnsFromScope(cteScope)
 		b.constructProjectForScope(cteScope, projectionsScope)

pkg/sql/opt/optbuilder/testdata/insert

@@ -671,7 +671,7 @@ with &1 (cte)
 build
 WITH cte AS (INSERT INTO abcde VALUES (1)) SELECT * FROM cte
 ----
-error (0A000): WITH clause "cte" does not have a RETURNING clause
+error (0A000): WITH clause "cte" does not return any columns
 
 # Use SRF in RETURNING clause.
 build

pkg/sql/opt/optbuilder/with.go

@@ -0,0 +1,51 @@
+// Copyright 2019 The Cockroach Authors.
+//
+// Use of this software is governed by the Business Source License
+// included in the file licenses/BSL.txt.
+//
+// As of the Change Date specified in that file, in accordance with
+// the Business Source License, use of this software will be governed
+// by the Apache License, Version 2.0, included in the file
+// licenses/APL.txt.
+
+package optbuilder
+
+import (
+	"github.com/cockroachdb/cockroach/pkg/sql/opt/props/physical"
+	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
+	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
+	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
+	"github.com/cockroachdb/errors"
+)
+
+// getCTECols renames a presentation for the scope, renaming the columns to
+// those provided in the AliasClause (if any). Throws an error if there is a
+// mistmatch in the number of columns.
+func (b *Builder) getCTECols(cteScope *scope, name tree.AliasClause) physical.Presentation {
+	presentation := cteScope.makePresentation()
+
+	if len(presentation) == 0 {
+		err := pgerror.Newf(
+			pgcode.FeatureNotSupported,
+			"WITH clause %q does not return any columns",
+			tree.ErrString(&name),
+		)
+		panic(errors.WithHint(err, "missing RETURNING clause?"))
+	}
+
+	if name.Cols == nil {
+		return presentation
+	}
+
+	if len(presentation) != len(name.Cols) {
+		panic(pgerror.Newf(
+			pgcode.InvalidColumnReference,
+			"source %q has %d columns available but %d columns specified",
+			name.Alias, len(presentation), len(name.Cols),
+		))
+	}
+	for i := range presentation {
+		presentation[i].Alias = string(name.Cols[i])
+	}
+	return presentation
+}