Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
64225: opt: make placeholder fast path conditional on the estimated row count r=RaduBerinde a=RaduBerinde

#### opt: show statistics in placeholder-fast-path tests

This highlights a problem with the fast path: we use statistics that
are derived using placeholders, which are usually terrible.

Release note: None

#### opt: mark columns as constant for equality with placeholder

This commit improves the FD generation for Select when we have filters
like `x = $1`. Because these filters have placeholders, they do not
generate constraints (which is the normal mechanism for detecting
constant columns).

This improves the cardinality property and row count estimate. The
estimate will be used for the placeholder fast path.

Release note: None

#### opt: make placeholder fast path conditional on the estimated row count

This change is best explained by this comment:

```
// We are dealing with a memo that still contains placeholders. The statistics
// for such a memo can be wildly overestimated. Even if our plan is correct,
// the estimated row count for a scan is passed to the execution engine which
// uses it to make sizing decisions. Passing a very high count can affect
// performance significantly (see #64214). So we only use the fast path if the
// estimated row count is small; typically this will happen when we constrain
// columns that form a key and we know there will be at most one row.
```

Fixes #64214.

Release note (bug fix): fixed a performance regression for very simple
queries.

64229: sql: add CREATE STATISTICS regression test r=mgartner a=mgartner

This is a "forward-port" of regression tests from #64226. This commit
contains no code changes because the bug was already been fixed on
master by #59687.

Release note: None

64235: roachtest: update version map and fixtures r=darinpp a=darinpp

This commit adds the recently released 20.1.15
to the version map in PredecessorVersion.

Release note: None (testing change)

Co-authored-by: Radu Berinde <[email protected]>
Co-authored-by: Marcus Gartner <[email protected]>
Co-authored-by: Darin Peshev <[email protected]>
  • Loading branch information
4 people committed Apr 27, 2021
4 parents 2cd071a + 976118b + a077779 + 92b8e6f commit f32a986
Show file tree
Hide file tree
Showing 14 changed files with 1,191 additions and 1,107 deletions.
Binary file modified pkg/cmd/roachtest/fixtures/1/checkpoint-v20.1.tgz
Binary file not shown.
Binary file modified pkg/cmd/roachtest/fixtures/2/checkpoint-v20.1.tgz
Binary file not shown.
Binary file modified pkg/cmd/roachtest/fixtures/3/checkpoint-v20.1.tgz
Binary file not shown.
Binary file modified pkg/cmd/roachtest/fixtures/4/checkpoint-v20.1.tgz
Binary file not shown.
2 changes: 1 addition & 1 deletion pkg/cmd/roachtest/test_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -1198,7 +1198,7 @@ func PredecessorVersion(buildVersion version.Version) (string, error) {
// map.
verMap := map[string]string{
"21.1": "20.2.6",
"20.2": "20.1.13",
"20.2": "20.1.15",
"20.1": "19.2.11",
"19.2": "19.1.11",
"19.1": "2.1.9",
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/roachtest/versionupgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func runVersionUpgrade(ctx context.Context, t *test, c *cluster, buildVersion ve
// The version to create/update the fixture for. Must be released (i.e.
// can download it from the homepage); if that is not the case use the
// empty string which uses the local cockroach binary.
newV := "20.2.6"
newV := "20.1.15"
predV, err := PredecessorVersion(*version.MustParse("v" + newV))
if err != nil {
t.Fatal(err)
Expand Down
11 changes: 11 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/distsql_stats
Original file line number Diff line number Diff line change
Expand Up @@ -1080,3 +1080,14 @@ job.typedesc_schema_change.successful
job.schema_change.successful
job.create_stats.successful
job.auto_create_stats.successful

# Regression test for #63387. Stats collection should succeed when partial index
# predicates reference inverted-type columns.
statement ok
CREATE TABLE t63387 (
i INT,
j JSONB,
INDEX (i) WHERE j->>'a' = 'b'
);
INSERT INTO t63387 VALUES (1, '{}');
CREATE STATISTICS s FROM t63387;
22 changes: 15 additions & 7 deletions pkg/sql/opt/memo/logical_props_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -1457,21 +1457,29 @@ func (b *logicalPropsBuilder) buildFiltersItemProps(item *FiltersItem, scalar *p

// Functional Dependencies
// -----------------------
// Add constant columns. No need to add not null columns, because they
// are only relevant if there are lax FDs that can be made strict.
var constCols opt.ColSet
if scalar.Constraints != nil {
constCols := scalar.Constraints.ExtractConstCols(b.evalCtx)
scalar.FuncDeps.AddConstants(constCols)
constCols = scalar.Constraints.ExtractConstCols(b.evalCtx)
}

// Check for filter conjunct of the form: x = y.
if eq, ok := item.Condition.(*EqExpr); ok {
if leftVar, ok := eq.Left.(*VariableExpr); ok {
if rightVar, ok := eq.Right.(*VariableExpr); ok {
scalar.FuncDeps.AddEquivalency(leftVar.Col, rightVar.Col)
switch rhs := eq.Right.(type) {
case *VariableExpr:
// Filter conjunct of the form: x = y.
scalar.FuncDeps.AddEquivalency(leftVar.Col, rhs.Col)

case *PlaceholderExpr:
// Filter conjunct of the form x = $1. This filter cannot generate
// constraints, but still tell us that the column is constant.
constCols.Add(leftVar.Col)
}
}
}

// Add constant columns. No need to add not null columns, because they
// are only relevant if there are lax FDs that can be made strict.
scalar.FuncDeps.AddConstants(constCols)
}

func (b *logicalPropsBuilder) buildProjectionsItemProps(
Expand Down
23 changes: 11 additions & 12 deletions pkg/sql/opt/optbuilder/testdata/delete
Original file line number Diff line number Diff line change
Expand Up @@ -181,20 +181,19 @@ delete xyz
└── limit
├── columns: x:5!null y:6 z:7 crdb_internal_mvcc_timestamp:8 column9:9
├── internal-ordering: -9
├── sort
│ ├── columns: x:5!null y:6 z:7 crdb_internal_mvcc_timestamp:8 column9:9
├── project
│ ├── columns: column9:9 x:5!null y:6 z:7 crdb_internal_mvcc_timestamp:8
│ ├── ordering: -9
│ ├── limit hint: 2.00
│ └── project
│ ├── columns: column9:9 x:5!null y:6 z:7 crdb_internal_mvcc_timestamp:8
│ ├── select
│ │ ├── columns: x:5!null y:6 z:7 crdb_internal_mvcc_timestamp:8
│ │ ├── scan xyz
│ │ │ └── columns: x:5!null y:6 z:7 crdb_internal_mvcc_timestamp:8
│ │ └── filters
│ │ └── x:5 = $1
│ └── projections
│ └── y:6 + $2 [as=column9:9]
│ ├── select
│ │ ├── columns: x:5!null y:6 z:7 crdb_internal_mvcc_timestamp:8
│ │ ├── limit hint: 2.00
│ │ ├── scan xyz
│ │ │ └── columns: x:5!null y:6 z:7 crdb_internal_mvcc_timestamp:8
│ │ └── filters
│ │ └── x:5 = $1
│ └── projections
│ └── y:6 + $2 [as=column9:9]
└── 2


Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/opt/xform/optimizer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ func TestPlaceholderFastPath(t *testing.T) {
runDataDrivenTest(
t,
"testdata/placeholder-fast-path",
memo.ExprFmtHideStats|memo.ExprFmtHideCost|memo.ExprFmtHideRuleProps|
memo.ExprFmtHideCost|memo.ExprFmtHideRuleProps|
memo.ExprFmtHideQualifications|memo.ExprFmtHideScalars|memo.ExprFmtHideTypes,
)
}
Expand Down
25 changes: 19 additions & 6 deletions pkg/sql/opt/xform/placeholder_fast_path.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import (
"github.com/cockroachdb/errors"
)

const maxRowCountForPlaceholderFastPath = 10

// TryPlaceholderFastPath attempts to produce a fully optimized memo with
// placeholders. This is only possible in very simple cases and involves special
// operators (PlaceholderScan) which use placeholders and resolve them at
Expand Down Expand Up @@ -47,13 +49,24 @@ func (o *Optimizer) TryPlaceholderFastPath() (_ opt.Expr, ok bool, err error) {
}()

root := o.mem.RootExpr().(memo.RelExpr)
rootProps := o.mem.RootProps()

if !rootProps.Ordering.Any() {
rootRelProps := root.Relational()
// We are dealing with a memo that still contains placeholders. The statistics
// for such a memo can be wildly overestimated. Even if our plan is correct,
// the estimated row count for a scan is passed to the execution engine which
// uses it to make sizing decisions. Passing a very high count can affect
// performance significantly (see #64214). So we only use the fast path if the
// estimated row count is small; typically this will happen when we constrain
// columns that form a key and we know there will be at most one row.
if rootRelProps.Stats.RowCount > maxRowCountForPlaceholderFastPath {
return nil, false, nil
}

rootColumns := root.Relational().OutputCols
rootPhysicalProps := o.mem.RootProps()

if !rootPhysicalProps.Ordering.Any() {
return nil, false, nil
}

// TODO(radu): if we want to support more cases, we should use optgen to write
// the rules.
Expand Down Expand Up @@ -175,7 +188,7 @@ func (o *Optimizer) TryPlaceholderFastPath() (_ opt.Expr, ok bool, err error) {

// Success!
newPrivate := scan.ScanPrivate
newPrivate.Cols = rootColumns
newPrivate.Cols = rootRelProps.OutputCols
newPrivate.Index = foundIndex.Ordinal()

span := make(memo.ScalarListExpr, numConstrained)
Expand All @@ -198,8 +211,8 @@ func (o *Optimizer) TryPlaceholderFastPath() (_ opt.Expr, ok bool, err error) {
ScanPrivate: newPrivate,
}
placeholderScan = o.mem.AddPlaceholderScanToGroup(placeholderScan, root)
o.mem.SetBestProps(placeholderScan, rootProps, &physical.Provided{}, 1.0 /* cost */)
o.mem.SetRoot(placeholderScan, rootProps)
o.mem.SetBestProps(placeholderScan, rootPhysicalProps, &physical.Provided{}, 1.0 /* cost */)
o.mem.SetRoot(placeholderScan, rootPhysicalProps)

if util.CrdbTestBuild && !o.mem.IsOptimized() {
return nil, false, errors.AssertionFailedf("IsOptimized() should be true")
Expand Down
120 changes: 63 additions & 57 deletions pkg/sql/opt/xform/testdata/external/hibernate
Original file line number Diff line number Diff line change
Expand Up @@ -308,27 +308,27 @@ inner-join (cross)
├── columns: id1_4_0_:1!null id1_2_1_:8!null address2_4_0_:2!null createdo3_4_0_:3 name4_4_0_:4 nickname5_4_0_:5 version6_4_0_:6!null name2_2_1_:9!null version3_2_1_:10!null
├── has-placeholder
├── key: (1,8)
├── fd: ()-->(10), (1)-->(2-6), (8)-->(9)
├── fd: ()-->(2,10), (1)-->(3-6), (8)-->(9)
├── project
│ ├── columns: person0_.id:1!null address:2!null createdon:3 person0_.name:4 nickname:5 person0_.version:6!null
│ ├── has-placeholder
│ ├── key: (1)
│ ├── fd: (1)-->(2-6)
│ ├── fd: ()-->(2), (1)-->(3-6)
│ └── inner-join (lookup person [as=person0_])
│ ├── columns: person0_.id:1!null address:2!null createdon:3 person0_.name:4 nickname:5 person0_.version:6!null person_id:15!null
│ ├── key columns: [15] = [1]
│ ├── lookup columns are key
│ ├── has-placeholder
│ ├── key: (15)
│ ├── fd: (1)-->(2-6), (1)==(15), (15)==(1)
│ ├── fd: ()-->(2), (1)-->(3-6), (1)==(15), (15)==(1)
│ ├── distinct-on
│ │ ├── columns: person_id:15
│ │ ├── grouping columns: person_id:15
│ │ ├── key: (15)
│ │ └── scan phone [as=phones2_]
│ │ └── columns: person_id:15
│ └── filters
│ └── address:2 = $1 [outer=(2), constraints=(/2: (/NULL - ])]
│ └── address:2 = $1 [outer=(2), constraints=(/2: (/NULL - ]), fd=()-->(2)]
├── select
│ ├── columns: partner1_.id:8!null partner1_.name:9!null partner1_.version:10!null
│ ├── has-placeholder
Expand Down Expand Up @@ -739,7 +739,7 @@ project
├── columns: phone0_.id:1!null phone_number:2 phone_type:3 person_id:4 max:12!null
├── has-placeholder
├── key: (1)
├── fd: (1)-->(2-4,12)
├── fd: ()-->(12), (1)-->(2-4)
├── group-by
│ ├── columns: phone0_.id:1!null phone_number:2 phone_type:3 person_id:4 max:12!null
│ ├── grouping columns: phone0_.id:1!null
Expand Down Expand Up @@ -770,7 +770,7 @@ project
│ └── const-agg [as=person_id:4, outer=(4)]
│ └── person_id:4
└── filters
└── max:12 = $1 [outer=(12), constraints=(/12: (/NULL - ])]
└── max:12 = $1 [outer=(12), constraints=(/12: (/NULL - ]), fd=()-->(12)]

opt
select
Expand Down Expand Up @@ -860,7 +860,7 @@ project
├── columns: phone0_.id:1!null phone_number:2 phone_type:3 person_id:4 min:12!null
├── has-placeholder
├── key: (1)
├── fd: (1)-->(2-4,12)
├── fd: ()-->(12), (1)-->(2-4)
├── group-by
│ ├── columns: phone0_.id:1!null phone_number:2 phone_type:3 person_id:4 min:12!null
│ ├── grouping columns: phone0_.id:1!null
Expand Down Expand Up @@ -891,7 +891,7 @@ project
│ └── const-agg [as=person_id:4, outer=(4)]
│ └── person_id:4
└── filters
└── min:12 = $1 [outer=(12), constraints=(/12: (/NULL - ])]
└── min:12 = $1 [outer=(12), constraints=(/12: (/NULL - ]), fd=()-->(12)]

opt
select
Expand Down Expand Up @@ -983,30 +983,33 @@ project
├── has-placeholder
├── key: (1)
├── fd: (1)-->(2-6)
└── inner-join (lookup person [as=person0_])
└── project
├── columns: person0_.id:1!null address:2 createdon:3 name:4 nickname:5 version:6!null person_id:11!null
├── key columns: [11] = [1]
├── lookup columns are key
├── cardinality: [0 - 1]
├── has-placeholder
├── key: (11)
├── fd: (1)-->(2-6), (1)==(11), (11)==(1)
├── distinct-on
│ ├── columns: person_id:11
│ ├── grouping columns: person_id:11
│ ├── has-placeholder
│ ├── key: (11)
│ └── select
│ ├── columns: phones1_.id:8!null person_id:11
│ ├── has-placeholder
│ ├── key: (8)
│ ├── fd: (8)-->(11)
│ ├── scan phone [as=phones1_]
│ │ ├── columns: phones1_.id:8!null person_id:11
│ │ ├── key: (8)
│ │ └── fd: (8)-->(11)
│ └── filters
│ └── phones1_.id:8 = $1 [outer=(8), constraints=(/8: (/NULL - ])]
└── filters (true)
├── key: ()
├── fd: ()-->(1-6,11), (1)==(11), (11)==(1)
└── inner-join (lookup person [as=person0_])
├── columns: person0_.id:1!null address:2 createdon:3 name:4 nickname:5 version:6!null phones1_.id:8!null person_id:11!null
├── key columns: [11] = [1]
├── lookup columns are key
├── cardinality: [0 - 1]
├── has-placeholder
├── key: ()
├── fd: ()-->(1-6,8,11), (11)==(1), (1)==(11)
├── select
│ ├── columns: phones1_.id:8!null person_id:11
│ ├── cardinality: [0 - 1]
│ ├── has-placeholder
│ ├── key: ()
│ ├── fd: ()-->(8,11)
│ ├── scan phone [as=phones1_]
│ │ ├── columns: phones1_.id:8!null person_id:11
│ │ ├── key: (8)
│ │ └── fd: (8)-->(11)
│ └── filters
│ └── phones1_.id:8 = $1 [outer=(8), constraints=(/8: (/NULL - ]), fd=()-->(8)]
└── filters (true)

opt
select
Expand All @@ -1033,30 +1036,33 @@ project
├── has-placeholder
├── key: (1)
├── fd: (1)-->(2-6)
└── inner-join (lookup person [as=person0_])
└── project
├── columns: person0_.id:1!null address:2 createdon:3 name:4 nickname:5 version:6!null person_id:11!null
├── key columns: [11] = [1]
├── lookup columns are key
├── cardinality: [0 - 1]
├── has-placeholder
├── key: (11)
├── fd: (1)-->(2-6), (1)==(11), (11)==(1)
├── distinct-on
│ ├── columns: person_id:11
│ ├── grouping columns: person_id:11
│ ├── has-placeholder
│ ├── key: (11)
│ └── select
│ ├── columns: phones1_.id:8!null person_id:11
│ ├── has-placeholder
│ ├── key: (8)
│ ├── fd: (8)-->(11)
│ ├── scan phone [as=phones1_]
│ │ ├── columns: phones1_.id:8!null person_id:11
│ │ ├── key: (8)
│ │ └── fd: (8)-->(11)
│ └── filters
│ └── phones1_.id:8 = $1 [outer=(8), constraints=(/8: (/NULL - ])]
└── filters (true)
├── key: ()
├── fd: ()-->(1-6,11), (1)==(11), (11)==(1)
└── inner-join (lookup person [as=person0_])
├── columns: person0_.id:1!null address:2 createdon:3 name:4 nickname:5 version:6!null phones1_.id:8!null person_id:11!null
├── key columns: [11] = [1]
├── lookup columns are key
├── cardinality: [0 - 1]
├── has-placeholder
├── key: ()
├── fd: ()-->(1-6,8,11), (11)==(1), (1)==(11)
├── select
│ ├── columns: phones1_.id:8!null person_id:11
│ ├── cardinality: [0 - 1]
│ ├── has-placeholder
│ ├── key: ()
│ ├── fd: ()-->(8,11)
│ ├── scan phone [as=phones1_]
│ │ ├── columns: phones1_.id:8!null person_id:11
│ │ ├── key: (8)
│ │ └── fd: (8)-->(11)
│ └── filters
│ └── phones1_.id:8 = $1 [outer=(8), constraints=(/8: (/NULL - ]), fd=()-->(8)]
└── filters (true)

opt
select
Expand Down Expand Up @@ -1865,17 +1871,17 @@ project
├── columns: auctioni4_1_0_:4!null id1_1_0_:1!null id1_1_1_:1!null amount2_1_1_:2 createdd3_1_1_:3 auctioni4_1_1_:4!null formula41_1_:11!null
├── has-placeholder
├── key: (1)
├── fd: (1)-->(2-4,11)
├── fd: ()-->(4), (1)-->(2,3,11)
├── group-by
│ ├── columns: bids0_.id:1!null amount:2 createddatetime:3 auctionid:4!null true_agg:13
│ ├── grouping columns: bids0_.id:1!null
│ ├── has-placeholder
│ ├── key: (1)
│ ├── fd: (1)-->(2-4,13)
│ ├── fd: ()-->(4), (1)-->(2-4,13)
│ ├── right-join (hash)
│ │ ├── columns: bids0_.id:1!null amount:2 createddatetime:3 auctionid:4!null successfulbid:9 true:12
│ │ ├── has-placeholder
│ │ ├── fd: (1)-->(2-4)
│ │ ├── fd: ()-->(4), (1)-->(2,3)
│ │ ├── project
│ │ │ ├── columns: true:12!null successfulbid:9
│ │ │ ├── fd: ()-->(12)
Expand All @@ -1887,13 +1893,13 @@ project
│ │ │ ├── columns: bids0_.id:1!null amount:2 createddatetime:3 auctionid:4!null
│ │ │ ├── has-placeholder
│ │ │ ├── key: (1)
│ │ │ ├── fd: (1)-->(2-4)
│ │ │ ├── fd: ()-->(4), (1)-->(2,3)
│ │ │ ├── scan tbid2 [as=bids0_]
│ │ │ │ ├── columns: bids0_.id:1!null amount:2 createddatetime:3 auctionid:4
│ │ │ │ ├── key: (1)
│ │ │ │ └── fd: (1)-->(2-4)
│ │ │ └── filters
│ │ │ └── auctionid:4 = $1 [outer=(4), constraints=(/4: (/NULL - ])]
│ │ │ └── auctionid:4 = $1 [outer=(4), constraints=(/4: (/NULL - ]), fd=()-->(4)]
│ │ └── filters
│ │ └── successfulbid:9 = bids0_.id:1 [outer=(1,9), constraints=(/1: (/NULL - ]; /9: (/NULL - ]), fd=(1)==(9), (9)==(1)]
│ └── aggregations
Expand Down
Loading

0 comments on commit f32a986

Please sign in to comment.