Skip to content

Commit

Permalink
opt: remove panic when in between filters are not constrained
Browse files Browse the repository at this point in the history
Before this change, the optimizer assumed that if partition filters are
constrained on an index then the in-between filters are also
constrained, and would panic if it was unable to find a constraint.

However, when an index is partitioned on multiple columns and the
columns have different orders, the optimizer may be unable to generate a
constraint for the in between filters of the partitions, even if the
partition filters are constrained. This change removes the panic, since
the assumption does not hold. As a consequence, the optimizer abondons
the GenerateConstrainedScan rule, but doesn't crash.

I did not include a release note since this appears to be a rare bug
that we have not encountered in the wild.

Fixes cockroachdb#80820

Release note: None
  • Loading branch information
rharding6373 committed May 18, 2022
1 parent a35b848 commit 235bb73
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 5 deletions.
3 changes: 1 addition & 2 deletions pkg/cmd/roachtest/tests/sqlsmith.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,11 +253,10 @@ INSERT INTO seed_mr_table DEFAULT VALUES;`, regionList[0]),
es := err.Error()
if strings.Contains(es, "internal error") {
// TODO(yuzefovich): we temporarily ignore internal errors
// that are because of #40929 and #80820.
// that are because of #40929.
var expectedError bool
for _, exp := range []string{
"could not parse \"0E-2019\" as type decimal",
"in-between filters didn't yield a constraint",
} {
expectedError = expectedError || strings.Contains(es, exp)
}
Expand Down
42 changes: 39 additions & 3 deletions pkg/sql/opt/xform/select_funcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/types"
"github.com/cockroachdb/cockroach/pkg/util"
"github.com/cockroachdb/errors"
)

// IsLocking returns true if the ScanPrivate is configured to use a row-level
Expand Down Expand Up @@ -239,6 +238,38 @@ func (c *CustomFuncs) MakeCombinedFiltersConstraint(
//
// Notice how we 'skip' all the europe-west2 rows with seq_num < 100.
//
// If there are multiple partitioning columns, the optimizer may be unable to
// generate a constraint for the in between filters, even if the partition
// filters are themselves constrained.
//
// Consider the following index and its partition:
//
// CREATE INDEX orders_by_seq_num
// ON orders (region ASC, zone DESC, seq_num, id)
// STORING (total)
// PARTITION BY LIST (region, zone)
// (
// PARTITION us_east1_a VALUES IN ('us-east1', 'zone-a'),
// PARTITION us_east1_b VALUES IN ('us-east1', 'zone-b'),
// PARTITION europe_west2_a VALUES IN ('europe-west2', 'zone-a')
// PARTITION europe_west2_b VALUES IN ('europe-west2', 'zone-b')
// )
//
// The constraint generated for the query:
// SELECT sum(total) FROM orders WHERE seq_num >= 100 AND seq_num < 200
// is:
// [/'europe-west2'/'zone-a'/100 - /'europe-west2'/'zone-a'/199]
// [/'europe-west2'/'zone-b'/100 - /'europe-west2'/'zone-b'/199]
// [/'us-east1'/'zone-a'/100 - /'us-east1'/'zone-a'/199]
// [/'us-east1'/'zone-b'/100 - /'us-east1'/'zone-b'/199]
//
// However, since region and zone are in ascending and descending order,
// respectively, the optimizer is currently unable to represent a span with
// columns in opposing directions and drops the last two fields from the span.
// This yields an unconstrained span.
//
// TODO(#81456): Add support for constrained in between filters when columns
// are in opposing order to fix the above problem.
var inBetweenFilters memo.FiltersExpr

indexColumns := tabMeta.IndexKeyColumns(index.Ordinal())
Expand Down Expand Up @@ -268,7 +299,12 @@ func (c *CustomFuncs) MakeCombinedFiltersConstraint(
index.Ordinal(),
)
if !ok {
panic(errors.AssertionFailedf("in-between filters didn't yield a constraint"))
// If there are multiple partitioning columns on the index with different
// orders, then we may not find a constraint even though the partition
// filters were constrained.
// TODO(#81456): Add support for constraints on multiple partitioning
// columns.
return nil, nil, nil, false
}

combinedConstraint.UnionWith(c.e.evalCtx, inBetweenConstraint)
Expand Down Expand Up @@ -406,7 +442,7 @@ func (c *CustomFuncs) GenerateConstrainedScans(
newScanPrivate.Cols = indexCols.Intersection(scanPrivate.Cols)
newScanPrivate.SetConstraint(c.e.evalCtx, combinedConstraint)
// Record whether we were able to use partitions to constrain the scan.
newScanPrivate.PartitionConstrainedScan = (len(partitionFilters) > 0)
newScanPrivate.PartitionConstrainedScan = len(partitionFilters) > 0

// If the alternate index includes the set of needed columns, then
// construct a new Scan operator using that index.
Expand Down
48 changes: 48 additions & 0 deletions pkg/sql/opt/xform/testdata/rules/select
Original file line number Diff line number Diff line change
Expand Up @@ -9330,3 +9330,51 @@ project
│ └── s:4
└── const-agg [as=b:5, outer=(5)]
└── b:5

# Reproduction of #80820.
exec-ddl
CREATE TABLE table80820 (
col0
INT8 NOT NULL,
col1
INT8 NOT NULL,
col2
INT8 NOT NULL,
col3
BYTES NOT NULL,
col4
CHAR NOT NULL,
INDEX (col0 DESC, col1, col2)
PARTITION BY LIST (col0, col1)
(
PARTITION one VALUES IN ((1, 10)),
PARTITION two VALUES IN ((2, 20))
),
INDEX (col3, col4 DESC, col2)
PARTITION BY LIST (col3, col4)
(
PARTITION one VALUES IN (('\xab', 'A')),
PARTITION two VALUES IN (('\xcd', 'C'))
)
)
----

opt
SELECT col0, col1, col2 FROM table80820 WHERE col2 < 4
----
select
├── columns: col0:1!null col1:2!null col2:3!null
├── scan table80820@table80820_col0_col1_col2_idx
│ └── columns: col0:1!null col1:2!null col2:3!null
└── filters
└── col2:3 < 4 [outer=(3), constraints=(/3: (/NULL - /3]; tight)]

opt
SELECT col3, col4, col2 FROM table80820 WHERE col2 < 4
----
select
├── columns: col3:4!null col4:5!null col2:3!null
├── scan table80820@table80820_col3_col4_col2_idx
│ └── columns: col2:3!null col3:4!null col4:5!null
└── filters
└── col2:3 < 4 [outer=(3), constraints=(/3: (/NULL - /3]; tight)]

0 comments on commit 235bb73

Please sign in to comment.