sqlsmith: make order-dependent aggregation functions deterministic

[michae2]

cmd: add smith command

Add a new top-level command smith which dumps randomly-generated
sqlsmith queries. This is useful for testing modifications to sqlsmith.

Assists: #83024

Release note: None

sqlsmith: make order-dependent aggregation functions deterministic

Some aggregation functions (e.g. string_agg) have results that depend
on the order of input rows. To make sqlsmith more deterministic, add
ORDER BY clauses to these aggregation functions whenever their argument
is a column reference. (When their argument is a constant, ordering
doesn't matter.)

Fixes: #83024

Release note: None

[cockroach-teamcity]

This change is 

[msirek]

Thanks for making the new smith command! The fix looks good.

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

---

pkg/cmd/smith/main.go line 35 at r2 (raw file):

	for {
		fmt.Print("\n", smither.Generate(), ";\n")

Maybe in the future we can add a command-line option to print a set number of statements.

[michae2]

TFYR!

bors r=msirek

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

---

pkg/cmd/smith/main.go line 35 at r2 (raw file):

Previously, msirek (Mark Sirek) wrote…

Maybe in the future we can add a command-line option to print a set number of statements.

Done. Also added some help text.

[craig]

Build failed:

• GitHub CI (Cockroach)

[mgartner]

pkg/cmd/smith/main.go line 65 at r3 (raw file):

	// TODO(michae2): Set smither options from command-line options.
	smithOpts := []sqlsmith.SmitherOption{}
	smither, err := sqlsmith.NewSmither(nil, rng, smithOpts...)

IIRC sqlsmith looks at the existing table schema to generate queries. What schema is used ins this case?

[mgartner]

The new smith command will be very useful!

I am slightly worried that the second commit (along with other recent changes) are reducing the efficacy of sqlsmith's in achieving its original goal - to find tests cases that cause panics. The more we constrain the output of sqlsmith to better support randomized correctness testing, the less effective it will become at randomized panic testing. Maybe should consider implementing two SQLSmith modes - one with almost no restrictions that can be used to catch panics, and one that is as restrictive as necessary to get deterministic results in randomized correctness tests.

Reviewed 4 of 6 files at r1, 3 of 3 files at r3, 1 of 1 files at r4, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @michae2 and @msirek)

[msirek]

Maybe we can set a flag when running any of the tests which compare results, and only use the ORDER BY if that flag is true.

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

[michae2]

Good point!

bors r-

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

---

pkg/cmd/smith/main.go line 65 at r3 (raw file):

Previously, mgartner (Marcus Gartner) wrote…

IIRC sqlsmith looks at the existing table schema to generate queries. What schema is used ins this case?

public

[michae2]

@mgartner @msirek how would you feel if I extended DisableImpureFns() to also cover this?

[mgartner]

public

Sorry, my question was not specific. I'm curious what tables SQLSmith is generating statements for when running this command. It doesn't seem like any tables would exist, unless they are being created somewhere here that I'm missing.

@mgartner @msirek how would you feel if I extended DisableImpureFns() to also cover this?

Sounds good to me. Maybe rename it to reflect it's increase influence? DisableNonDeterminism?

[msirek]

+1

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

[michae2]

Ok, I went hog-wild and added to smith:

• passthrough of simple sqlsmith options
• option to connect to a database for schema info
• new MutatingMode for sqlsmith based on mutating smither from TLP / costfuzz / unoptimized-query-oracle
• ability to generate expressions instead of statements

To answer your question, @mgartner, without a db connection sqlsmith can only generate expressions, SELECT statements (over VALUES tables), CREATE TABLE statements, and probably a few other things. But no mutations.

As for the original issue, I extended DisableImpureFns to cover adding the ORDER BY and renamed it to DisableNondeterministicFns.

Since I changed so much, I'll wait for another look whenever you have time (no rush).

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

[msirek]

Updates look good

[michae2]

Thanks!

bors r=msirek,mgartner

[craig]

Build succeeded:

• GitHub CI (Cockroach)