Skip to content

Commit ed045f3

Browse files
committed
randgen: fix recent regression in tuple generation
In 1a751b5 we made a slight change to how we generate random tuples. Namely, we no longer call `ResolvedType` on each tuple element. This can be problematic since we might have generated an element with some Any modifier, which could later lead to "illegal AppendFloat/FormatFloat bitSize" panic when operating with floats. This commit simply reverts the relevant changes from the commit mentioned above. This required some minor adjustments to `TestCanonical` since there tuple with NULL elements will have their resolved type differ from the originally requested one. Release note: None
1 parent 223617d commit ed045f3

File tree

2 files changed

+33
-4
lines changed

2 files changed

+33
-4
lines changed

pkg/sql/randgen/datum.go

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -239,16 +239,21 @@ func RandDatumWithNullChance(
239239
}
240240
return tree.NewDJsonpath(*jp.AST)
241241
case types.TupleFamily:
242+
tuple := tree.DTuple{D: make(tree.Datums, len(typ.TupleContents()))}
242243
if nullChance == 0 {
244+
// Even if nullOk=false (which is when nullChance=0), we still want
245+
// to generate a NULL _element_ in 10% of cases.
243246
nullChance = 10
244247
}
245-
datums := make([]tree.Datum, len(typ.TupleContents()))
246248
for i := range typ.TupleContents() {
247-
datums[i] = RandDatumWithNullChance(
249+
tuple.D[i] = RandDatumWithNullChance(
248250
rng, typ.TupleContents()[i], nullChance, favorCommonData, targetColumnIsUnique,
249251
)
250252
}
251-
return tree.NewDTuple(typ, datums...)
253+
// Calling ResolvedType causes the internal TupleContents types to be
254+
// populated (as well as resolves the type of each tuple element).
255+
tuple.ResolvedType()
256+
return &tuple
252257
case types.BitFamily:
253258
width := typ.Width()
254259
if width == 0 {

pkg/sql/randgen/types_test.go

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"fmt"
1010
"testing"
1111

12+
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
1213
"github.com/cockroachdb/cockroach/pkg/sql/types"
1314
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
1415
"github.com/cockroachdb/cockroach/pkg/util/randutil"
@@ -66,7 +67,30 @@ func TestCanonical(t *testing.T) {
6667
datum := RandDatum(rng, typ, false)
6768
datumTyp := datum.ResolvedType()
6869
if !datumTyp.Equivalent(typ.Canonical()) {
69-
t.Errorf("fail: canonical type of %+v is %+v and the datum's type is %+v", typ, typ.Canonical(), datumTyp)
70+
var tupleHasNullElement func(tree.Datum) bool
71+
tupleHasNullElement = func(d tree.Datum) bool {
72+
tup, ok := d.(*tree.DTuple)
73+
if !ok {
74+
return false
75+
}
76+
for _, el := range tup.D {
77+
if el == tree.DNull {
78+
return true
79+
}
80+
if tupleHasNullElement(el) {
81+
return true
82+
}
83+
}
84+
return false
85+
}
86+
// If we have a tuple, then we might have included a NULL element.
87+
// By construction in RandDatum, TupleContents[i] has been updated
88+
// to types.Unknown. In such case, we give an exception and don't
89+
// fail the test.
90+
tupleException := tupleHasNullElement(datum)
91+
if !tupleException {
92+
t.Errorf("fail: canonical type of %+v is %+v and the datum's type is %+v", typ, typ.Canonical(), datumTyp)
93+
}
7094
}
7195

7296
if datumTyp.Oid() != typ.Canonical().Oid() {

0 commit comments

Comments
 (0)