sqlsmith: improvements #37456
sqlsmith: improvements #37456
Conversation
|
This change is |
This will be used when we want to set some initial state and not have to worry about it. Useful for comparison testing across separate servers to avoid having to make sure changes are synced. Release note: None
|
ping @justinj |
There was a problem hiding this comment.
Something I think might be cool with these PRs would be to have a datadriven test that generated a handful of statements (maybe 10, maybe 100), possibly parameterized by the options framework you've added here. It could be seeded so pure refactorings wouldn't change it (up to like, reorderings of random calls, which I think is fine). There would obviously still be a lot of churn in that file but having a handful of examples of what we generate now included directly in a PR would be really nice when looking at code changes.
Reviewed 2 of 2 files at r1, 2 of 2 files at r2, 1 of 1 files at r3, 1 of 1 files at r4, 1 of 1 files at r5, 1 of 1 files at r6.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @mjibson)
pkg/internal/sqlsmith/relational.go, line 336 at r5 (raw file):
} func makeSelectTable(s *scope, refs colRefs, forJoin bool) (tree.TableExpr, colRefs, bool) {
I think this function could use a comment
pkg/internal/sqlsmith/relational.go, line 384 at r5 (raw file):
var fromRefs colRefs // Sometimes generate a SELECT with no FROM clause. for coin() {
this seems like a high percentage of the time to me but I have no actual evidence for that
pkg/internal/sqlsmith/relational.go, line 759 at r5 (raw file):
values, valuesRefs := makeValues(s, desiredTypes, refs) // Returing just &values here would result in a query like `VALUES (...)` where
nit: Returing (present in the old version as well)
pkg/internal/sqlsmith/sqlsmith.go, line 125 at r1 (raw file):
} // SmitherOption is an option for the Smither client.
this is a cute pattern!
It can now use VALUES and SELECT as table sources. Also teach it about having no FROM sources at all.
There was a problem hiding this comment.
Yes, I've wanted this too. I want to do it in a way that is fairly deterministic and doesn't cause churn here because someone moved or added a function or operator in the tree package. I'll think about this more.
Reviewable status:
pkg/internal/sqlsmith/relational.go, line 336 at r5 (raw file):
Previously, justinj (Justin Jaffray) wrote…
I think this function could use a comment
Done.
pkg/internal/sqlsmith/relational.go, line 384 at r5 (raw file):
Previously, justinj (Justin Jaffray) wrote…
this seems like a high percentage of the time to me but I have no actual evidence for that
Done.
|
bors r+ |
37456: sqlsmith: improvements r=mjibson a=mjibson Various SQLsmith improvements in preparation for comparison testing and correlated subqueries. Release note: None Co-authored-by: Matt Jibson <[email protected]>
Build succeeded |
Various SQLsmith improvements in preparation for comparison testing and correlated subqueries.
Release note: None