Skip to content

Commit

Permalink
sql/stats: use nil *eval.Context as CompareContext when forecasting
Browse files Browse the repository at this point in the history
When forecasting table statistics, we don't need a full *eval.Context.
We can simply use a nil *eval.Context as a tree.CompareContext. This
means we don't have to plumb an eval.Context into the stats cache.

Assists: cockroachdb#79872

Release note: None
  • Loading branch information
michae2 committed Aug 15, 2022
1 parent 33b343d commit 135c976
Show file tree
Hide file tree
Showing 6 changed files with 17 additions and 24 deletions.
2 changes: 2 additions & 0 deletions pkg/sql/rowexec/sample_aggregator.go
Original file line number Diff line number Diff line change
Expand Up @@ -616,6 +616,8 @@ func (s *sampleAggregator) generateHistogram(
prevCapacity, sr.Cap(),
)
}
// TODO(michae2): Instead of using the flowCtx's evalCtx, investigate
// whether this can use a nil *eval.Context.
h, _, err := stats.EquiDepthHistogram(evalCtx, colType, values, numRows, distinctCount, maxBuckets)
return h, err
}
Expand Down
3 changes: 3 additions & 0 deletions pkg/sql/sem/eval/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,9 @@ func (ec *Context) SetStmtTimestamp(ts time.Time) {

// GetLocation returns the session timezone.
func (ec *Context) GetLocation() *time.Location {
if ec == nil {
return time.UTC
}
return ec.SessionData().GetLocation()
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/show_stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ func (p *planner) ShowTableStats(ctx context.Context, n *tree.ShowTableStats) (p
observed[i], observed[j] = observed[j], observed[i]
}

forecasts := stats.ForecastTableStatistics(ctx, p.EvalContext(), observed)
forecasts := stats.ForecastTableStatistics(ctx, observed)

// Iterate in reverse order to match the ORDER BY "columnIDs".
for i := len(forecasts) - 1; i >= 0; i-- {
Expand Down
20 changes: 6 additions & 14 deletions pkg/sql/stats/forecast.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (

"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
"github.com/cockroachdb/cockroach/pkg/sql/opt/cat"
"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/types"
"github.com/cockroachdb/cockroach/pkg/util/log"
Expand Down Expand Up @@ -57,12 +58,7 @@ const maxForecastDistance = time.Hour * 24 * 7
//
// ForecastTableStatistics is deterministic: given the same observations it will
// return the same forecasts.
//
// TODO(michae2): Use nil *eval.Context or custom tree.CompareContext instead of
// taking an evalCtx.
func ForecastTableStatistics(
ctx context.Context, evalCtx tree.CompareContext, observed []*TableStatistic,
) []*TableStatistic {
func ForecastTableStatistics(ctx context.Context, observed []*TableStatistic) []*TableStatistic {
// Early sanity check. We'll check this again in forecastColumnStatistics.
if len(observed) < minObservationsForForecast {
return nil
Expand Down Expand Up @@ -102,9 +98,7 @@ func ForecastTableStatistics(

forecasts := make([]*TableStatistic, 0, len(forecastCols))
for _, colKey := range forecastCols {
forecast, err := forecastColumnStatistics(
ctx, evalCtx, observedByCols[colKey], at, minGoodnessOfFit,
)
forecast, err := forecastColumnStatistics(ctx, observedByCols[colKey], at, minGoodnessOfFit)
if err != nil {
log.VEventf(
ctx, 2, "could not forecast statistics for table %v columns %s: %v",
Expand Down Expand Up @@ -135,11 +129,7 @@ func ForecastTableStatistics(
// forecastColumnStatistics is deterministic: given the same observations and
// forecast time, it will return the same forecast.
func forecastColumnStatistics(
ctx context.Context,
evalCtx tree.CompareContext,
observed []*TableStatistic,
at time.Time,
minRequiredFit float64,
ctx context.Context, observed []*TableStatistic, at time.Time, minRequiredFit float64,
) (forecast *TableStatistic, err error) {
if len(observed) < minObservationsForForecast {
return nil, errors.New("not enough observations to forecast statistics")
Expand Down Expand Up @@ -263,6 +253,8 @@ func forecastColumnStatistics(
// histogram. NOTE: If any of the observed histograms were for inverted
// indexes this will produce an incorrect histogram.
if observed[0].HistogramData != nil {
var evalCtx *eval.Context

hist, err := predictHistogram(
ctx, evalCtx, observed, forecastAt, minRequiredFit, nonNullRowCount,
)
Expand Down
5 changes: 1 addition & 4 deletions pkg/sql/stats/forecast_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,8 @@ import (
"time"

"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
"github.com/cockroachdb/cockroach/pkg/sql/sem/catid"
"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"
"github.com/cockroachdb/cockroach/pkg/sql/types"
"github.com/cockroachdb/cockroach/pkg/util/timeutil"
)
Expand Down Expand Up @@ -581,7 +579,6 @@ func TestForecastColumnStatistics(t *testing.T) {
},
}
ctx := context.Background()
evalCtx := eval.NewTestingEvalContext(cluster.MakeTestingClusterSettings())
for i, tc := range testCases {
t.Run(strconv.Itoa(i), func(t *testing.T) {

Expand All @@ -593,7 +590,7 @@ func TestForecastColumnStatistics(t *testing.T) {
expected := tc.forecast.toTableStatistic(jobspb.ForecastStatsName, i)
at := testStatTime(tc.at)

forecast, err := forecastColumnStatistics(ctx, evalCtx, observed, at, 1)
forecast, err := forecastColumnStatistics(ctx, observed, at, 1)
if err != nil {
if !tc.err {
t.Errorf("test case %d unexpected forecastColumnStatistics err: %v", i, err)
Expand Down
9 changes: 4 additions & 5 deletions pkg/sql/stats/quantile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"strconv"
"testing"

"github.com/cockroachdb/cockroach/pkg/settings/cluster"
"github.com/cockroachdb/cockroach/pkg/sql/opt/cat"
"github.com/cockroachdb/cockroach/pkg/sql/sem/eval"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
Expand All @@ -41,7 +40,7 @@ func TestRandomQuantileRoundTrip(t *testing.T) {
types.Float4,
}
colTypes = append(colTypes, types.Scalar...)
evalCtx := eval.NewTestingEvalContext(cluster.MakeTestingClusterSettings())
var evalCtx *eval.Context
rng, seed := randutil.NewTestRand()
for _, colType := range colTypes {
if canMakeQuantile(histVersion, colType) {
Expand Down Expand Up @@ -566,7 +565,7 @@ func TestQuantileToHistogram(t *testing.T) {
err: true,
},
}
evalCtx := eval.NewTestingEvalContext(cluster.MakeTestingClusterSettings())
var evalCtx *eval.Context
for i, tc := range testCases {
t.Run(strconv.Itoa(i), func(t *testing.T) {
hist, err := tc.qfun.toHistogram(evalCtx, types.Float, tc.rows)
Expand Down Expand Up @@ -843,7 +842,7 @@ func TestQuantileValueRoundTrip(t *testing.T) {
err: true,
},
}
evalCtx := eval.NewTestingEvalContext(cluster.MakeTestingClusterSettings())
var evalCtx *eval.Context
for i, tc := range testCases {
t.Run(strconv.Itoa(i), func(t *testing.T) {
val, err := toQuantileValue(tc.dat)
Expand Down Expand Up @@ -1120,7 +1119,7 @@ func TestQuantileValueRoundTripOverflow(t *testing.T) {
res: quantileMaxTimestampSec,
},
}
evalCtx := eval.NewTestingEvalContext(cluster.MakeTestingClusterSettings())
var evalCtx *eval.Context
for i, tc := range testCases {
t.Run(strconv.Itoa(i), func(t *testing.T) {
d, err := fromQuantileValue(tc.typ, tc.val)
Expand Down

0 comments on commit 135c976

Please sign in to comment.