Merge pull request #47716 from RaduBerinde/backport19.2-47715

release-19.2: opt: fix in-place modification of window definition in optbuilder

pkg/sql/logictest/testdata/logic_test/views

@@ -537,3 +537,24 @@ query T
 SELECT create_statement FROM [SHOW CREATE w3]
 ----
 CREATE VIEW w3 (x) AS (WITH t AS (SELECT x FROM test.public.w) SELECT x FROM t)
+
+# Regression test for #47704: the columns inside PARITION BY and ORDER BY were
+# losing their qualification.
+statement ok
+CREATE TABLE a47704 (foo UUID);
+CREATE TABLE b47704 (foo UUID)
+
+statement ok
+CREATE VIEW v47704 AS
+  SELECT first_value(a47704.foo) OVER (PARTITION BY a47704.foo ORDER BY a47704.foo)
+  FROM a47704 JOIN b47704 ON a47704.foo = b47704.foo
+
+# Verify that the descriptor did not "lose" the column qualification inside
+# PARITION BY and ORDER BY.
+query T
+SELECT create_statement FROM [ SHOW CREATE VIEW v47704 ]
+----
+CREATE VIEW v47704 (first_value) AS SELECT first_value(a47704.foo) OVER (PARTITION BY a47704.foo ORDER BY a47704.foo) FROM test.public.a47704 JOIN test.public.b47704 ON a47704.foo = b47704.foo
+
+statement ok
+SELECT * FROM v47704

pkg/sql/opt/optbuilder/scope.go

@@ -1026,7 +1026,7 @@ func (s *scope) lookupWindowDef(name tree.Name) *tree.WindowDef {
 	panic(pgerror.Newf(pgcode.UndefinedObject, "window %q does not exist", name))
 }
 
-func (s *scope) constructWindowDef(def tree.WindowDef) *tree.WindowDef {
+func (s *scope) constructWindowDef(def tree.WindowDef) tree.WindowDef {
 	switch {
 	case def.RefName != "":
 		// SELECT rank() OVER (w) FROM t WINDOW w AS (...)
@@ -1035,14 +1035,16 @@ func (s *scope) constructWindowDef(def tree.WindowDef) *tree.WindowDef {
 		if err != nil {
 			panic(err)
 		}
-		return &result
+		return result
+
 	case def.Name != "":
 		// SELECT rank() OVER w FROM t WINDOW w AS (...)
 		// Note the lack of parens around w, compared to the first case.
 		// We use the referenced window specification directly, without modification.
-		return s.lookupWindowDef(def.Name)
+		return *s.lookupWindowDef(def.Name)
+
 	default:
-		return &def
+		return def
 	}
 }
 
@@ -1061,9 +1063,12 @@ func (s *scope) replaceWindowFn(f *tree.FuncExpr, def *tree.FunctionDefinition)
 	s.builder.semaCtx.Properties.Require("window",
 		tree.RejectNestedWindowFunctions)
 
-	f.WindowDef = s.constructWindowDef(*f.WindowDef)
+	// Make a copy of f so we can modify the WindowDef.
+	fCopy := *f
+	newWindowDef := s.constructWindowDef(*f.WindowDef)
+	fCopy.WindowDef = &newWindowDef
 
-	expr := f.Walk(s)
+	expr := fCopy.Walk(s)
 
 	typedFunc, err := tree.TypeCheck(expr, s.builder.semaCtx, types.Any)
 	if err != nil {
@@ -1088,17 +1093,25 @@ func (s *scope) replaceWindowFn(f *tree.FuncExpr, def *tree.FunctionDefinition)
 	)
 	s.builder.semaCtx.Properties.Derived.InWindowFunc = true
 
-	for i, e := range f.WindowDef.Partitions {
+	oldPartitions := f.WindowDef.Partitions
+	f.WindowDef.Partitions = make(tree.Exprs, len(oldPartitions))
+	for i, e := range oldPartitions {
 		typedExpr := s.resolveType(e, types.Any)
 		f.WindowDef.Partitions[i] = typedExpr
 	}
-	for i, e := range f.WindowDef.OrderBy {
-		if e.OrderType != tree.OrderByColumn {
+
+	oldOrderBy := f.WindowDef.OrderBy
+	f.WindowDef.OrderBy = make(tree.OrderBy, len(oldOrderBy))
+	for i := range oldOrderBy {
+		ord := *oldOrderBy[i]
+		if ord.OrderType != tree.OrderByColumn {
 			panic(errOrderByIndexInWindow)
 		}
-		typedExpr := s.resolveType(e.Expr, types.Any)
-		f.WindowDef.OrderBy[i].Expr = typedExpr
+		typedExpr := s.resolveType(ord.Expr, types.Any)
+		ord.Expr = typedExpr
+		f.WindowDef.OrderBy[i] = &ord
 	}
+
 	if f.WindowDef.Frame != nil {
 		if err := analyzeWindowFrame(s, f.WindowDef); err != nil {
 			panic(err)

pkg/sql/sem/tree/select.go

@@ -819,13 +819,13 @@ const (
 // OverrideWindowDef implements the logic to have a base window definition which
 // then gets augmented by a different window definition.
 func OverrideWindowDef(base *WindowDef, override WindowDef) (WindowDef, error) {
-	// referencedSpec.Partitions is always used.
+	// base.Partitions is always used.
 	if len(override.Partitions) > 0 {
 		return WindowDef{}, pgerror.Newf(pgcode.Windowing, "cannot override PARTITION BY clause of window %q", base.Name)
 	}
 	override.Partitions = base.Partitions
 
-	// referencedSpec.OrderBy is used if set.
+	// base.OrderBy is used if set.
 	if len(base.OrderBy) > 0 {
 		if len(override.OrderBy) > 0 {
 			return WindowDef{}, pgerror.Newf(pgcode.Windowing, "cannot override ORDER BY clause of window %q", base.Name)