sqlsmith: add support for computed columns#61491
sqlsmith: add support for computed columns#61491craig[bot] merged 2 commits intocockroachdb:masterfrom
Conversation
|
This change is |
mgartner
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @jordanlewis and @RaduBerinde)
pkg/sql/rowenc/testutils.go, line 1245 at r1 (raw file):
// Make defs for computed columns. normalColDefs := columnDefs for i := nNormalColumns; i < nColumns; i++ {
[nit] i is not used in the block, so for i := 0; i < nComputedColumns; i++ { might be easier to understand.
pkg/sql/rowenc/testutils.go, line 1536 at r1 (raw file):
if rng.Intn(2) == 0 { // Try to find a set of numeric columns with the same type so we can add // them together as the computed expression.
[nit] expand this explanation similar to the single-column explanation below. An example might be nice.
pkg/sql/rowenc/testutils.go, line 1594 at r1 (raw file):
case types.DecimalFamily: var err error right, err = tree.ParseDDecimal("1")
[nit] I think you could use RandDatum to get a random "special" datum for right, rather than hard-coding to 1.
mgartner
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/rowenc/testutils.go, line 1245 at r1 (raw file):
Previously, mgartner (Marcus Gartner) wrote…
[nit]
iis not used in the block, sofor i := 0; i < nComputedColumns; i++ {might be easier to understand.
nvm, i is used. disregard.
a395586 to
6a8b39a
Compare
RaduBerinde
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/rowenc/testutils.go, line 1594 at r1 (raw file):
Previously, mgartner (Marcus Gartner) wrote…
[nit] I think you could use
RandDatumto get a random "special" datum forright, rather than hard-coding to1.
Good idea, that also made the code simpler.
6a8b39a to
2809314
Compare
|
Had to make a few fixes: don't create FKs from computed columns or to virtual columns (which we don't support); and use |
mgartner
left a comment
There was a problem hiding this comment.
Reviewed 2 of 2 files at r2, 1 of 1 files at r3.
Reviewable status:
pkg/sql/rowenc/testutils.go, line 1621 at r2 (raw file):
}, True: tree.NewStrVal("foo"), Else: tree.NewStrVal("bar"),
[nit] you could use RandDatum here too
This changes the random table generator to also create computed columns (either STORED or VIRTUAL). Some example of definitions: - `col1_14 STRING NOT NULL AS (lower(CAST(col1_8 AS STRING))) VIRTUAL` - `col1_10 FLOAT8 AS (col1_5 + (-0.16607712222551002):::FLOAT8) STORED` - `col1_13 INT4 AS (col1_0 + col1_10) STORED` Release justification: non-production code change. Release note: None
Fixing this test to use FmtParsable. Without it, an expression that contains an `Infinity` decimal isn't quoted properly. Release justification: non-production code change Release note: None
2809314 to
d9949a3
Compare
RaduBerinde
left a comment
There was a problem hiding this comment.
TFTR!
Reviewable status:
pkg/sql/rowenc/testutils.go, line 1621 at r2 (raw file):
Previously, mgartner (Marcus Gartner) wrote…
[nit] you could use RandDatum here too
Done.
mgartner
left a comment
There was a problem hiding this comment.
Reviewed 2 of 2 files at r4, 1 of 1 files at r5.
Reviewable status:
|
bors r+ |
|
Build succeeded: |
This changes the random table generator to also create computed
columns (either STORED or VIRTUAL). Some example of definitions:
col1_14 STRING NOT NULL AS (lower(CAST(col1_8 AS STRING))) VIRTUALcol2_6 DECIMAL NOT NULL AS (col2_2 + 1:::DECIMAL) STOREDcol1_13 INT4 AS (col1_0 + col1_10) STOREDRelease justification: non-production code change.
Release note: None
Informs #57608.