Skip to content

Commit

Permalink
opt: unexport relational stats to avoid accidental copies
Browse files Browse the repository at this point in the history
Previously, the `props.Statistics` field of `props.Relational` was exported.
This made it easy to accidentally copy it, which could show up on heap profiles
because `props.Statistics` is a fairly large struct.

This patch unexports `props.Statistics` and exposes a `Relational.Statistics()`
method that returns a pointer to the field. This changes the default behavior
to reference the struct, rather than copying it, in order to prevent accidental
copies in the future (and revert the current ones).

Release note: None
  • Loading branch information
DrewKimball committed Sep 19, 2022
1 parent 36d7a21 commit 8389ac4
Show file tree
Hide file tree
Showing 23 changed files with 226 additions and 195 deletions.
2 changes: 1 addition & 1 deletion pkg/sql/opt/exec/execbuilder/builder.go
Expand Up @@ -226,7 +226,7 @@ func (b *Builder) Build() (_ exec.Plan, err error) {
return nil, err
}

rootRowCount := int64(b.e.(memo.RelExpr).Relational().Stats.RowCountIfAvailable())
rootRowCount := int64(b.e.(memo.RelExpr).Relational().Statistics().RowCountIfAvailable())
return b.factory.ConstructPlan(plan.root, b.subqueries, b.cascades, b.checks, rootRowCount)
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/opt/exec/execbuilder/cascades.go
Expand Up @@ -241,7 +241,7 @@ func (cb *cascadeBuilder) planCascade(
Min: uint32(numBufferedRows),
Max: uint32(numBufferedRows),
}
bindingProps.Stats = props.Statistics{
*bindingProps.Statistics() = props.Statistics{
Available: true,
RowCount: float64(numBufferedRows),
}
Expand Down
20 changes: 10 additions & 10 deletions pkg/sql/opt/exec/execbuilder/relational.go
Expand Up @@ -388,7 +388,7 @@ func (b *Builder) buildRelational(e memo.RelExpr) (execPlan, error) {
// ExplainFactory and annotates the node with more information if so.
func (b *Builder) maybeAnnotateWithEstimates(node exec.Node, e memo.RelExpr) {
if ef, ok := b.factory.(exec.ExplainFactory); ok {
stats := &e.Relational().Stats
stats := e.Relational().Statistics()
val := exec.EstimatedStats{
TableStatsAvailable: stats.Available,
RowCount: stats.RowCount,
Expand Down Expand Up @@ -616,8 +616,8 @@ func (b *Builder) scanParams(
// been removed by DetachMemo. Update that function if the column stats are
// needed here in the future.
var rowCount float64
if relProps.Stats.Available {
rowCount = relProps.Stats.RowCount
if relProps.Statistics().Available {
rowCount = relProps.Statistics().RowCount
}

if scan.PartitionConstrainedScan {
Expand Down Expand Up @@ -715,7 +715,7 @@ func (b *Builder) buildScan(scan *memo.ScanExpr) (execPlan, error) {

// Save if we planned a full table/index scan on the builder so that the
// planner can be made aware later. We only do this for non-virtual tables.
stats := scan.Relational().Stats
stats := scan.Relational().Statistics()
if !tab.IsVirtualTable() && isUnfiltered {
large := !stats.Available || stats.RowCount > b.evalCtx.SessionData().LargeFullScanRows
if scan.Index == cat.PrimaryIndex {
Expand Down Expand Up @@ -1167,8 +1167,8 @@ func (b *Builder) buildHashJoin(join memo.RelExpr) (execPlan, error) {
// it during the costing.
// TODO(raduberinde): we might also need to look at memo.JoinFlags when
// choosing a side.
leftRowCount := leftExpr.Relational().Stats.RowCount
rightRowCount := rightExpr.Relational().Stats.RowCount
leftRowCount := leftExpr.Relational().Statistics().RowCount
rightRowCount := rightExpr.Relational().Statistics().RowCount
if leftRowCount < rightRowCount {
if joinType == descpb.LeftSemiJoin {
joinType = descpb.RightSemiJoin
Expand Down Expand Up @@ -1252,8 +1252,8 @@ func (b *Builder) buildMergeJoin(join *memo.MergeJoinExpr) (execPlan, error) {
// it during the costing.
// TODO(raduberinde): we might also need to look at memo.JoinFlags when
// choosing a side.
leftRowCount := leftExpr.Relational().Stats.RowCount
rightRowCount := rightExpr.Relational().Stats.RowCount
leftRowCount := leftExpr.Relational().Statistics().RowCount
rightRowCount := rightExpr.Relational().Statistics().RowCount
if leftRowCount < rightRowCount {
if joinType == descpb.LeftSemiJoin {
joinType = descpb.RightSemiJoin
Expand Down Expand Up @@ -2578,7 +2578,7 @@ func (b *Builder) buildWith(with *memo.WithExpr) (execPlan, error) {
// to behave like a spoolNode) and using the EXISTS mode.
Mode: exec.SubqueryAllRows,
Root: buffer,
RowCount: int64(with.Relational().Stats.RowCountIfAvailable()),
RowCount: int64(with.Relational().Statistics().RowCountIfAvailable()),
})

b.addBuiltWithExpr(with.ID, value.outputCols, buffer)
Expand Down Expand Up @@ -2632,7 +2632,7 @@ func (b *Builder) buildRecursiveCTE(rec *memo.RecursiveCTEExpr) (execPlan, error
if err != nil {
return nil, err
}
rootRowCount := int64(rec.Recursive.Relational().Stats.RowCountIfAvailable())
rootRowCount := int64(rec.Recursive.Relational().Statistics().RowCountIfAvailable())
return innerBld.factory.ConstructPlan(plan.root, innerBld.subqueries, innerBld.cascades, innerBld.checks, rootRowCount)
}

Expand Down
8 changes: 4 additions & 4 deletions pkg/sql/opt/exec/execbuilder/scalar.go
Expand Up @@ -498,7 +498,7 @@ func (b *Builder) buildArrayFlatten(
typ := b.mem.Metadata().ColumnMeta(af.RequestedCol).Type
e := b.addSubquery(
exec.SubqueryAllRows, typ, root.root, af.OriginalExpr,
int64(af.Input.Relational().Stats.RowCountIfAvailable()),
int64(af.Input.Relational().Statistics().RowCountIfAvailable()),
)

return tree.NewTypedArrayFlattenExpr(e), nil
Expand Down Expand Up @@ -555,7 +555,7 @@ func (b *Builder) buildAny(ctx *buildScalarCtx, scalar opt.ScalarExpr) (tree.Typ
typs := types.MakeTuple(contents)
subqueryExpr := b.addSubquery(
exec.SubqueryAnyRows, typs, plan.root, any.OriginalExpr,
int64(any.Input.Relational().Stats.RowCountIfAvailable()),
int64(any.Input.Relational().Statistics().RowCountIfAvailable()),
)

// Build the scalar value that is compared against each row.
Expand Down Expand Up @@ -591,7 +591,7 @@ func (b *Builder) buildExistsSubquery(

return b.addSubquery(
exec.SubqueryExists, types.Bool, plan.root, exists.OriginalExpr,
int64(exists.Input.Relational().Stats.RowCountIfAvailable()),
int64(exists.Input.Relational().Statistics().RowCountIfAvailable()),
), nil
}

Expand Down Expand Up @@ -621,7 +621,7 @@ func (b *Builder) buildSubquery(

return b.addSubquery(
exec.SubqueryOneRow, subquery.Typ, plan.root, subquery.OriginalExpr,
int64(input.Relational().Stats.RowCountIfAvailable()),
int64(input.Relational().Statistics().RowCountIfAvailable()),
), nil
}

Expand Down
3 changes: 2 additions & 1 deletion pkg/sql/opt/lookupjoin/constraint_builder_test.go
Expand Up @@ -278,7 +278,8 @@ func makeFilters(
}

// Create a fake Select and input so that normalization rules are run.
p := &props.Relational{OutputCols: cols, Cardinality: card, Stats: stats}
p := &props.Relational{OutputCols: cols, Cardinality: card}
*p.Statistics() = stats
fakeRel := f.ConstructFakeRel(&memo.FakeRelPrivate{Props: p})
sel := f.ConstructSelect(fakeRel, filters)

Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/opt/memo/expr_format.go
Expand Up @@ -797,9 +797,9 @@ func (f *ExprFmtCtx) formatRelational(e RelExpr, tp treeprinter.Node) {

if !f.HasFlags(ExprFmtHideStats) {
if f.HasFlags(ExprFmtHideHistograms) {
tp.Childf("stats: %s", relational.Stats.StringWithoutHistograms())
tp.Childf("stats: %s", relational.Statistics().StringWithoutHistograms())
} else {
tp.Childf("stats: %s", &relational.Stats)
tp.Childf("stats: %s", relational.Statistics())
}
}

Expand Down
10 changes: 5 additions & 5 deletions pkg/sql/opt/memo/logical_props_builder.go
Expand Up @@ -938,7 +938,7 @@ func (b *logicalPropsBuilder) buildWithProps(with *WithExpr, rel *props.Relation
// Statistics
// ----------
// Inherited from the input expression.
rel.Stats = inputProps.Stats
*rel.Statistics() = *inputProps.Statistics()
}

func (b *logicalPropsBuilder) buildWithScanProps(withScan *WithScanExpr, rel *props.Relational) {
Expand Down Expand Up @@ -2134,7 +2134,7 @@ func ensureLookupJoinInputProps(join *LookupJoinExpr, sb *statisticsBuilder) *pr
relational.Cardinality = props.AnyCardinality
relational.FuncDeps.CopyFrom(MakeTableFuncDep(md, join.Table))
relational.FuncDeps.ProjectCols(relational.OutputCols)
relational.Stats = *sb.makeTableStatistics(join.Table)
*relational.Statistics() = *sb.makeTableStatistics(join.Table)
}
return relational
}
Expand All @@ -2156,7 +2156,7 @@ func ensureInvertedJoinInputProps(join *InvertedJoinExpr, sb *statisticsBuilder)
relational.FuncDeps.ProjectCols(relational.OutputCols)

// TODO(rytaft): Change this to use inverted index stats once available.
relational.Stats = *sb.makeTableStatistics(join.Table)
*relational.Statistics() = *sb.makeTableStatistics(join.Table)
}
return relational
}
Expand Down Expand Up @@ -2201,7 +2201,7 @@ func ensureInputPropsForIndex(
relProps.Cardinality = props.AnyCardinality
relProps.FuncDeps.CopyFrom(MakeTableFuncDep(md, tabID))
relProps.FuncDeps.ProjectCols(relProps.OutputCols)
relProps.Stats = *sb.makeTableStatistics(tabID)
*relProps.Statistics() = *sb.makeTableStatistics(tabID)
}
}

Expand Down Expand Up @@ -2624,7 +2624,7 @@ func (b *logicalPropsBuilder) buildMemoCycleTestRelProps(
// failures.
rel.OutputCols = inputProps.OutputCols
// Make the row count non-zero to avoid assertion failures.
rel.Stats.RowCount = 1
rel.Statistics().RowCount = 1
}

// WithUses returns the WithUsesMap for the given expression.
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/opt/memo/memo.go
Expand Up @@ -487,7 +487,7 @@ func (m *Memo) Detach() {

switch t := parent.(type) {
case RelExpr:
t.Relational().Stats.ColStats = props.ColStatsMap{}
t.Relational().Statistics().ColStats = props.ColStatsMap{}
}
}
clearColStats(m.RootExpr())
Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/opt/memo/memo_test.go
Expand Up @@ -362,7 +362,7 @@ func TestStatsAvailable(t *testing.T) {

testNotAvailable := func(expr memo.RelExpr) {
traverseExpr(expr, func(e memo.RelExpr) {
if e.Relational().Stats.Available {
if e.Relational().Statistics().Available {
t.Fatal("stats should not be available")
}
})
Expand Down Expand Up @@ -400,7 +400,7 @@ func TestStatsAvailable(t *testing.T) {

testAvailable := func(expr memo.RelExpr) {
traverseExpr(expr, func(e memo.RelExpr) {
if !e.Relational().Stats.Available {
if !e.Relational().Statistics().Available {
t.Fatal("stats should be available")
}
})
Expand Down

0 comments on commit 8389ac4

Please sign in to comment.