Skip to content

Commit

Permalink
opt: nil presentation should not equal the 0-column presentation
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
RaduBerinde committed Aug 22, 2019
1 parent 91351fd commit 9d04015
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 5 deletions.
12 changes: 8 additions & 4 deletions pkg/sql/opt/memo/interner.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
4 changes: 4 additions & 0 deletions pkg/sql/opt/props/physical/required.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
10 changes: 9 additions & 1 deletion pkg/sql/opt/props/physical/required_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 9d04015

Please sign in to comment.