From 9d0401506f23094f4fe91bfe68b1b57eb33ff933 Mon Sep 17 00:00:00 2001
From: Radu Berinde <radu@cockroachlabs.com>
Date: Thu, 22 Aug 2019 09:26:26 -0400
Subject: [PATCH] opt: nil presentation should not equal the 0-column
 presentation

The nil presentation means that we want all columns in no particular
order, whereas the 0-column presentation means that we don't need any
of the columns (e.g. EXISTS subqueries). Only the nil presentation
is `Any()`. But `Equals()` was incorrectly treating these as equal.
This confused the interner and a nil presentation became a 0-column
presentation which led to some internal errors in Apply.

For completeness, we also fix `HashPhysProps` to differentiate
between these two, but note that this isn't required for correctness.

Fixes #39435.

Release note (bug fix): Fixed internal errors generated during
execution of some complicated cases of correlated subqueries.
---
 pkg/sql/opt/memo/interner.go                | 12 ++++++++----
 pkg/sql/opt/props/physical/required.go      |  4 ++++
 pkg/sql/opt/props/physical/required_test.go | 10 +++++++++-
 3 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/pkg/sql/opt/memo/interner.go b/pkg/sql/opt/memo/interner.go
index 19f010879fbc..4307af062535 100644
--- a/pkg/sql/opt/memo/interner.go
+++ b/pkg/sql/opt/memo/interner.go
@@ -522,10 +522,14 @@ func (h *hasher) HashTupleOrdinal(val TupleOrdinal) {
 }
 
 func (h *hasher) HashPhysProps(val *physical.Required) {
-	for i := range val.Presentation {
-		col := &val.Presentation[i]
-		h.HashString(col.Alias)
-		h.HashColumnID(col.ID)
+	// Note: the Any presentation is not the same as the 0-column presentation.
+	if !val.Presentation.Any() {
+		h.HashInt(len(val.Presentation))
+		for i := range val.Presentation {
+			col := &val.Presentation[i]
+			h.HashString(col.Alias)
+			h.HashColumnID(col.ID)
+		}
 	}
 	h.HashOrderingChoice(val.Ordering)
 }
diff --git a/pkg/sql/opt/props/physical/required.go b/pkg/sql/opt/props/physical/required.go
index 76af14526a94..53f633c99fa4 100644
--- a/pkg/sql/opt/props/physical/required.go
+++ b/pkg/sql/opt/props/physical/required.go
@@ -115,6 +115,10 @@ func (p Presentation) Any() bool {
 // Equals returns true iff this presentation exactly matches the given
 // presentation.
 func (p Presentation) Equals(rhs Presentation) bool {
+	// The 0 column presentation is not the same as the nil presentation.
+	if p.Any() != rhs.Any() {
+		return false
+	}
 	if len(p) != len(rhs) {
 		return false
 	}
diff --git a/pkg/sql/opt/props/physical/required_test.go b/pkg/sql/opt/props/physical/required_test.go
index b8b424387320..c6400958a286 100644
--- a/pkg/sql/opt/props/physical/required_test.go
+++ b/pkg/sql/opt/props/physical/required_test.go
@@ -42,10 +42,18 @@ func TestRequiredProps(t *testing.T) {
 		t.Error("presentation should equal itself")
 	}
 
-	if presentation.Equals(physical.Presentation{}) {
+	if presentation.Equals(physical.Presentation(nil)) {
 		t.Error("presentation should not equal the empty presentation")
 	}
 
+	if presentation.Equals(physical.Presentation{}) {
+		t.Error("presentation should not equal the 0 column presentation")
+	}
+
+	if (physical.Presentation{}).Equals(physical.Presentation(nil)) {
+		t.Error("0 column presentation should not equal the empty presentation")
+	}
+
 	// Add ordering props.
 	ordering := physical.ParseOrderingChoice("+1,+5")
 	phys.Ordering = ordering