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

opt: fix issue with dangling pointer in statisticsBuilder #37983

Merged
merged 1 commit into from
Jun 6, 2019
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
4 changes: 4 additions & 0 deletions pkg/sql/opt/memo/statistics_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,8 @@ func (sb *statisticsBuilder) colStatLeaf(
colStat.NullCount = s.RowCount * unknownNullCountRatio
} else {
colStatLeaf := sb.colStatLeaf(nullableCols, s, fd, notNullCols)
// Fetch the colStat again since it may now have a different address.
colStat, _ = s.ColStats.Lookup(colSet)
colStat.NullCount = colStatLeaf.NullCount
}
}
Expand Down Expand Up @@ -414,6 +416,8 @@ func (sb *statisticsBuilder) colStatLeaf(
nullCount += colStatLeaf.NullCount * (1 - nullCount/s.RowCount)
}
})
// Fetch the colStat again since it may now have a different address.
colStat, _ = s.ColStats.Lookup(colSet)
colStat.DistinctCount = min(distinctCount, s.RowCount)
colStat.NullCount = min(nullCount, s.RowCount)
}
Expand Down
92 changes: 92 additions & 0 deletions pkg/sql/opt/memo/testdata/stats/scan
Original file line number Diff line number Diff line change
Expand Up @@ -522,3 +522,95 @@ scan a
├── stats: [rows=10]
├── key: (1)
└── ordering: +1

# Regression test for #37953.
exec-ddl
CREATE TABLE t37953 (
a UUID NOT NULL,
b FLOAT8 NOT NULL,
c TIME NOT NULL,
d UUID NOT NULL,
e VARCHAR,
f "char" NULL,
g INT4 NOT NULL,
h VARCHAR NULL,
i REGPROC NULL,
j FLOAT8 NOT NULL
)
----
TABLE t37953
├── a uuid not null
├── b float not null
├── c time not null
├── d uuid not null
├── e varchar
├── f "char"
├── g int4 not null
├── h varchar
├── i regproc
├── j float not null
├── rowid int not null (hidden)
└── INDEX primary
└── rowid int not null (hidden)

norm
WITH
subq (col0, col1)
AS (
SELECT
tab1.g AS col0,
CASE
WHEN ilike_escape(
regexp_replace(
tab0.h,
tab1.e,
tab0.f,
tab0.e::STRING
),
tab1.f,
''
)
THEN true
ELSE false
END
AS col1
FROM
t37953 AS tab0, t37953 AS tab1
WHERE
tab0.j IN (tab1.j,)
)
SELECT
1
FROM
subq
WHERE
subq.col1;
----
project
├── columns: "?column?":24(int!null)
├── stats: [rows=0.95099005]
├── fd: ()-->(24)
├── select
│ ├── columns: col1:23(bool!null)
│ ├── stats: [rows=0.95099005, distinct(23)=0.95099005, null(23)=0]
│ ├── fd: ()-->(23)
│ ├── project
│ │ ├── columns: col1:23(bool)
│ │ ├── stats: [rows=333333.333, distinct(23)=333333.333, null(23)=16336.65]
│ │ ├── inner-join
│ │ │ ├── columns: tab0.e:5(varchar) tab0.f:6("char") tab0.h:8(varchar) tab0.j:10(float!null) tab1.e:16(varchar) tab1.f:17("char") tab1.j:21(float!null)
│ │ │ ├── stats: [rows=333333.333, distinct(10)=100, null(10)=0, distinct(21)=100, null(21)=0, distinct(5,6,8,16,17)=333333.333, null(5,6,8,16,17)=16336.65]
│ │ │ ├── scan tab0
│ │ │ │ ├── columns: tab0.e:5(varchar) tab0.f:6("char") tab0.h:8(varchar) tab0.j:10(float!null)
│ │ │ │ └── stats: [rows=1000, distinct(10)=100, null(10)=0, distinct(5,6,8)=1000, null(5,6,8)=29.701]
│ │ │ ├── scan tab1
│ │ │ │ ├── columns: tab1.e:16(varchar) tab1.f:17("char") tab1.j:21(float!null)
│ │ │ │ └── stats: [rows=1000, distinct(21)=100, null(21)=0, distinct(16,17)=1000, null(16,17)=19.9]
│ │ │ └── filters
│ │ │ └── tab0.j IN (tab1.j,) [type=bool, outer=(10,21)]
│ │ └── projections
│ │ └── CASE WHEN ilike_escape(regexp_replace(tab0.h, tab1.e, tab0.f, tab0.e::STRING), tab1.f, '') THEN true ELSE false END [type=bool, outer=(5,6,8,16,17)]
│ └── filters
│ └── variable: col1 [type=bool, outer=(23), constraints=(/23: [/true - /true]; tight), fd=()-->(23)]
└── projections
└── const: 1 [type=int]
12 changes: 12 additions & 0 deletions pkg/sql/opt/props/col_stats_map.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,10 @@ func (m *ColStatsMap) Count() int {

// Get returns the nth statistic in the map, by its ordinal position. This
// position is stable across calls to Get or Add (but not RemoveIntersecting).
// NOTE: The returned *ColumnStatistic is only valid until this ColStatsMap is
// updated via a call to Add() or RemoveIntersecting(). At that point,
// the address of the statistic may have changed, so it must be fetched
// again using another call to Get() or Lookup().
func (m *ColStatsMap) Get(nth int) *ColumnStatistic {
if nth < initialColStatsCap {
return &m.initial[nth]
Expand All @@ -107,6 +111,10 @@ func (m *ColStatsMap) Get(nth int) *ColumnStatistic {

// Lookup returns the column statistic indexed by the given column set. If no
// such statistic exists in the map, then ok=false.
// NOTE: The returned *ColumnStatistic is only valid until this ColStatsMap is
// updated via a call to Add() or RemoveIntersecting(). At that point,
// the address of the statistic may have changed, so it must be fetched
// again using another call to Lookup() or Get().
func (m *ColStatsMap) Lookup(cols opt.ColSet) (colStat *ColumnStatistic, ok bool) {
// Scan the inlined statistics if there are only a few statistics in the map.
if m.count <= initialColStatsCap {
Expand Down Expand Up @@ -150,6 +158,10 @@ func (m *ColStatsMap) Lookup(cols opt.ColSet) (colStat *ColumnStatistic, ok bool
// it does not yet exist in the map, then Add adds a new blank ColumnStatistic
// and returns it, along with added=true. Otherwise, Add returns the existing
// ColumnStatistic with added=false.
// NOTE: The returned *ColumnStatistic is only valid until this ColStatsMap is
// updated via another call to Add() or RemoveIntersecting(). At that
// point, the address of the statistic may have changed, so it must be
// fetched again using Lookup() or Get().
func (m *ColStatsMap) Add(cols opt.ColSet) (_ *ColumnStatistic, added bool) {
// Only add column set if it is not already present in the map.
colStat, ok := m.Lookup(cols)
Expand Down