Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

release-22.2: roachtest: make window functions deterministic #105672

Merged
merged 1 commit into from
Jun 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions pkg/cmd/roachtest/tests/query_comparison_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,9 +188,6 @@ func runOneRoundQueryComparison(
sqlsmith.UnlikelyRandomNulls(), sqlsmith.DisableCrossJoins(),
sqlsmith.DisableIndexHints(), sqlsmith.DisableWith(), sqlsmith.DisableDecimals(),
sqlsmith.LowProbabilityWhereClauseWithJoinTables(),
// TODO(mgartner): Re-enable aggregate functions when we can guarantee
// they are deterministic.
sqlsmith.DisableAggregateFuncs(),
sqlsmith.SetComplexity(.3),
sqlsmith.SetScalarComplexity(.1),
)
Expand Down
19 changes: 8 additions & 11 deletions pkg/internal/sqlsmith/scalar.go
Original file line number Diff line number Diff line change
Expand Up @@ -473,18 +473,15 @@ func makeFunc(s *Smither, ctx Context, typ *types.T, refs colRefs) (tree.TypedEx
})
if s.disableNondeterministicFns {
switch fn.def.Name {
case "lead", "lag":
// Lead and lag need all columns to be added to the ORDER BY because
// while they respect the order, they can return any value within the
// partition. This means we need to be consistent about which rows fall
// on the boundary between peer groups.
for _, ref := range refs {
addOrdCol(ref.typedExpr(), ref.typ)
}
case "rank", "dense_rank", "percent_rank":
// The rank functions don't need to add ordering columns because they
// return the same result for all rows in a given peer group.
default:
// Other window functions only need to order by the argument columns.
for _, arg := range args {
addOrdCol(arg, arg.ResolvedType())
// Other window functions care about the relative order of rows within
// each peer group, so we ensure that the ordering is deterministic by
// limiting each peer group to one row (by ordering on all columns).
for _, i := range s.rnd.Perm(len(refs)) {
addOrdCol(refs[i].typedExpr(), refs[i].typ)
}
}
}
Expand Down