Skip to content

Commit

Permalink
sql: fix crash in physicalProps
Browse files Browse the repository at this point in the history
The `addEquivalency` method wasn't properly handling the case when one
of the columns is constant. This causes the other column to become
constant, and thus it needs to be removed from all weak keys.

Adding a test designed to catch the general class of bugs where a
mutation causes the invariants to be invalidated; verified that it
catches the problem.

Fixes #24500.

Release note (bug fix): Fixed a crash caused by a WHERE condition that
requires a column to equal a specific value and at the same time equal
another column.
  • Loading branch information
RaduBerinde committed Apr 5, 2018
1 parent 2175975 commit b5554ee
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 12 deletions.
18 changes: 18 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/physical_props
Expand Up @@ -193,3 +193,21 @@ join 0 join · · (a, b, c, d,
└── scan 2 scan · · (e, f, g) e!=NULL; key(e); +e
· 2 · table efg@primary · ·
· 2 · spans ALL · ·

# Regression test for 24500
statement ok
CREATE TABLE abc (a INT, b INT, c INT);
CREATE UNIQUE INDEX ON abc (a, b, c);
SELECT true FROM abc WHERE b=1 and b=c

# a should be a weak key.
query TITTTTT colnames
EXPLAIN (VERBOSE) SELECT true FROM abc WHERE b=1 and b=c;
----
Tree Level Type Field Description Columns Ordering
render 0 render · · ("true") "true"=CONST
│ 0 · render 0 true · ·
└── scan 1 scan · · (a[omitted], b, c, rowid[hidden,omitted]) b=c; b=CONST; c!=NULL; weak-key(a)
· 1 · table abc@abc_a_b_c_key · ·
· 1 · spans ALL · ·
· 1 · filter (b = 1) AND (b = c) · ·
16 changes: 13 additions & 3 deletions pkg/sql/physical_props.go
Expand Up @@ -396,10 +396,20 @@ func (pp *physicalProps) addEquivalency(colA, colB int) {
pp.notNullCols.Add(gA)
}

for i := range pp.weakKeys {
if pp.weakKeys[i].Contains(gB) {
if pp.constantCols.Contains(gA) {
// One of the columns became a constant; remove it from all keys (similar to
// what addConstantColumn does).
for i := range pp.weakKeys {
pp.weakKeys[i].Remove(gA)
pp.weakKeys[i].Remove(gB)
pp.weakKeys[i].Add(gA)
}
} else {
// Replace any occurrences of gB with gA (the new representative).
for i := range pp.weakKeys {
if pp.weakKeys[i].Contains(gB) {
pp.weakKeys[i].Remove(gB)
pp.weakKeys[i].Add(gA)
}
}
}

Expand Down
53 changes: 44 additions & 9 deletions pkg/sql/physical_props_test.go
Expand Up @@ -377,13 +377,6 @@ func TestTrimOrderingGuarantee(t *testing.T) {
// ord.public.trim(desired)
// after := ord.computeMatch(desired)

genDir := func(rng *rand.Rand) encoding.Direction {
if rng.Intn(2) == 0 {
return encoding.Descending
}
return encoding.Ascending
}

for _, numConstCols := range []int{0, 1, 2, 4} {
for _, numEquiv := range []int{0, 1, 2, 4, 5} {
for _, numOrderCols := range []int{0, 1, 2, 4, 7} {
Expand Down Expand Up @@ -419,7 +412,7 @@ func TestTrimOrderingGuarantee(t *testing.T) {
if o.eqGroups.Find(x) == x && !used.Contains(x) {
used.Add(x)
keySet.Add(x)
o.addOrderColumn(x, genDir(rng))
o.addOrderColumn(x, randDir(rng))
break
}
}
Expand All @@ -434,7 +427,7 @@ func TestTrimOrderingGuarantee(t *testing.T) {
perm := rng.Perm(10)
for i := range desired {
desired[i].ColIdx = perm[i]
desired[i].Direction = genDir(rng)
desired[i].Direction = randDir(rng)
}
oCopy := o.copy()
before := oCopy.computeMatch(desired)
Expand Down Expand Up @@ -842,3 +835,45 @@ func TestProjectOrdering(t *testing.T) {
})
}
}

// TestRandomProps calls mutation functions randomly and verifies the invariants
// of the structure each time.
func TestRandomProps(t *testing.T) {
defer leaktest.AfterTest(t)()

for _, n := range []int{2, 5, 10} {
t.Run(fmt.Sprintf("%d", n), func(t *testing.T) {
t.Parallel()
rng, _ := randutil.NewPseudoRand()
for it := 0; it < 100; it++ {
o := physicalProps{}

for op := 0; op < 100; op++ {
switch rng.Intn(4) {
case 0:
o.addEquivalency(rng.Intn(n), rng.Intn(n))
case 1:
o.addConstantColumn(rng.Intn(n))
case 2:
o.addOrderColumn(rng.Intn(n), randDir(rng))
case 3:
var key util.FastIntSet
num := 1 + rng.Intn(n/2)
for i := 0; i < num; i++ {
key.Add(rng.Intn(n))
}
o.addWeakKey(key)
}
o.check()
}
}
})
}
}

func randDir(rng *rand.Rand) encoding.Direction {
if rng.Intn(2) == 0 {
return encoding.Descending
}
return encoding.Ascending
}

0 comments on commit b5554ee

Please sign in to comment.