sqlsmith: enhance tlp to test predicate projections and DISTINCTs

[jordanlewis]

This commit enhances the WHERE tlp generator to also output the
projection of the predicates in both the partitioned and unpartitioned
query outputs. The purpose of this enhanced testing is to make sure that
projection output always matches filter output, and that projection
output for all 3 partitions is identical in every partition and in the
non-partitioned case as well.

Also, add a DISTINCT oracle.

The DISTINCT oracle for TLP generates unpartitioned queries of the
following form:

SELECT DISTINCT {cols...} FROM t

and partitioned queries of the following form:

SELECT DISTINCT {cols...} FROM t WHERE pred UNION ALL
SELECT DISTINCT {cols...} FROM t WHERE NOT (pred) UNION ALL
SELECT DISTINCT {cols...} FROM t WHERE (pred) IS NULL

Release note: None

[cockroach-teamcity]

This change is 

[jordanlewis]

Hi @mgartner, I wrote this extra testing as a way to start understanding TLP a bit better. I think it does test some new things, since the path for projections is different from the path for filters. But, maybe it's not novel enough to add? I'm not sure, interested to see what you think.

[jordanlewis]

Second commit adds the DISTINCT oracle given by Manuel's paper.

[jordanlewis]

Gentle ping on this if you have some time to take a look!

[mgartner]

Very cool! Sorry for the delay. , just left some minor suggestions.

Reviewed 1 of 1 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @jordanlewis)

---

pkg/internal/sqlsmith/tlp.go, line 65 at r1 (raw file):

// The first query returned is an unpartitioned query of the form:
//
//   SELECT *, p, NOT (p), (p) IS NULL, true, false, false FROM table

I'm not sure the last three columns are necessary. Is there a type of bug you have in mind that they would catch that would not be caught by the addition of the first three columns?

---

pkg/internal/sqlsmith/tlp.go, line 109 at r1 (raw file):

FROM %s
WHERE %s`, allPreds, pred1, pred2, pred3, tableName, pred1)
	part2 := fmt.Sprintf(`SELECT *, 

nit: remove trailing whitespace

---

pkg/internal/sqlsmith/tlp.go, line 392 at r2 (raw file):

//
//   SELECT DISTINCT {cols...} FROM table WHERE (p) UNION ALL
//   SELECT DISTINCT {cols...} FROM table WHERE NOT (p) UNION ALL

If you change the UNION ALLs to UNIONs, then you don't need the to return a distinct boolean above and you can remove de-duplicating code from unsortedMatrixesDiff.

[jordanlewis]

Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @mgartner)

---

pkg/internal/sqlsmith/tlp.go, line 65 at r1 (raw file):

Previously, mgartner (Marcus Gartner) wrote…

I'm not sure the last three columns are necessary. Is there a type of bug you have in mind that they would catch that would not be caught by the addition of the first three columns?

I added this because if there's some way an entire column was be projected to the incorrect boolean, we'd have false passes. I guess it's an implausible case though?

---

pkg/internal/sqlsmith/tlp.go, line 392 at r2 (raw file):

Previously, mgartner (Marcus Gartner) wrote…

If you change the UNION ALLs to UNIONs, then you don't need the to return a distinct boolean above and you can remove de-duplicating code from unsortedMatrixesDiff.

Good point, done.

[mgartner]

Reviewed 2 of 2 files at r4, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @jordanlewis)

---

pkg/internal/sqlsmith/tlp.go, line 65 at r1 (raw file):

Previously, jordanlewis (Jordan Lewis) wrote…

I added this because if there's some way an entire column was be projected to the incorrect boolean, we'd have false passes. I guess it's an implausible case though?

🤷 Hard to say how plausible that is. I'm fine keeping it. Might be explaining the columns in this comment though.

[jordanlewis]

TFTR!

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @mgartner)

---

pkg/internal/sqlsmith/tlp.go, line 65 at r1 (raw file):

Previously, mgartner (Marcus Gartner) wrote…

🤷 Hard to say how plausible that is. I'm fine keeping it. Might be explaining the columns in this comment though.

Done.

[jordanlewis]

bors r+

[craig]

Build succeeded:

• GitHub CI (Cockroach)