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: add GeneratePartialIndexScans rule #50070

Merged
merged 1 commit into from Jun 15, 2020

Conversation

mgartner
Copy link
Collaborator

@mgartner mgartner commented Jun 10, 2020

This commit introduces a new exploration rule,
GeneratePartialIndexScans. This rule generates unconstrained Scans over
partial indexes when a Select filter implies the partial index
predicate.

For example, consider the table and query:

CREATE TABLE t (
  i INT,
  s STRING,
  INDEX pidx (i) STORING (s) WHERE s = 'foo'
)

SELECT i FROM t WHERE s = 'foo'

GeneratePartialIndexScans will generate a scan over the partial index:

project
 ├── columns: i:1
 └── scan t@pidx
      └── columns: i:1 s:3!null

It is capable of handling cases where the index does not cover the
selected columns, and where additional filtering is required after the
partial index scan. For example:

CREATE TABLE t (
  i INT,
  s STRING,
  f FLOAT,
  INDEX pidx (i) WHERE s = 'foo'
)

SELECT i FROM t WHERE s = 'foo' AND f = 2.5

In this case, GeneratePartialIndexScans will generate the following
expression with an added Select and IndexJoin:

project
 ├── columns: i:1
 └── index-join t
      ├── columns: i:1 s:2!null f:3!null
      └── select
           ├── columns: i:1 rowid:4!null
           ├── scan t@pidx
           │    └── columns: i:1 rowid:4!null
           └── filters
                └── f:3 = 2.5

Fixes #50232

Release note: None

@mgartner mgartner requested a review from a team as a code owner June 10, 2020 23:26
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@mgartner mgartner force-pushed the generate-partial-index-scans branch from 7be29ce to 0a8ca76 Compare June 11, 2020 00:53
Copy link
Member

@RaduBerinde RaduBerinde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm: Nice work!

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @mgartner)


pkg/sql/opt/xform/custom_funcs.go, line 134 at r2 (raw file):

// GeneratePartialIndexScans generates unconstrained index scans over all
// partial indexes with predicates that are implied by the filters. Partial
// indexes with predicates cannot be proven to be implied by the filters are

[nit] which cannot


pkg/sql/opt/xform/custom_funcs.go, line 138 at r2 (raw file):

//
// When a filter completely matches the predicate, the remaining filters are
// simplified so that they do not include the filter. A redundnat filter is

redundant


pkg/sql/opt/xform/custom_funcs.go, line 193 at r2 (raw file):

		// If index is covering and there are no remaining filters, construct a
		// new Scan in the same group as the original Select.
		if remainingFilters.ChildCount() == 0 && covering {

[nit] len(remainingFilters)


pkg/sql/opt/xform/custom_funcs.go, line 196 at r2 (raw file):

			scan := memo.ScanExpr{ScanPrivate: *scanPrivate}
			scan.Index = iter.IndexOrdinal()
			c.e.mem.AddScanToGroup(&scan, grp)

[nit] Doesn't indexScanBuilder cover this case too?


pkg/sql/opt/xform/custom_funcs.go, line 226 at r2 (raw file):

		// If there are remaining filters, construct a Select with them.
		if remainingFilters.ChildCount() != 0 {

We don't need this check (addSelect already checks it)


pkg/sql/opt/xform/partial_index.go, line 20 at r2 (raw file):

)

// predicateImpliedByFilters attempts to prove that a partial index predicate is

I think this is accidentally left over


pkg/sql/opt/xform/testdata/rules/select, line 223 at r2 (raw file):

      └── filters
           └── NOT b:4 [outer=(4), constraints=(/4: [/false - /false]; tight), fd=()-->(4)]

Maybe add a test where the table has multiple partial indexes (as well as some non-partial indexes).

Copy link
Collaborator

@rytaft rytaft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! :lgtm: once you address Radu's comments.

Reviewed 5 of 5 files at r1, 6 of 6 files at r2.
Reviewable status: :shipit: complete! 2 of 0 LGTMs obtained (waiting on @mgartner)


pkg/sql/opt/testutils/opttester/opt_tester.go, line 830 at r2 (raw file):

			return false
		}
		return true

While you're making changes here, probably worth adding the ruleName to ot.seenRules like we do in Optimize above


pkg/sql/opt/xform/custom_funcs.go, line 144 at r2 (raw file):

// For each partial index, there are two conditions to consider: whether or not
// the index "covers" the columns needed and whether or not the remaining filter
// is empty. There two conditions combine to create four possible cases:

There two -> These two


pkg/sql/opt/xform/custom_funcs.go, line 148 at r2 (raw file):

//   - The index is covering and the remaining filter is empty. In this case, A
//     single unconstrained Scan operator is added to the same group as the
//     original Select operator).

[nit] extra ")"


pkg/sql/opt/xform/custom_funcs.go, line 191 at r2 (raw file):

		covering := iter.IsCovering()

		// If index is covering and there are no remaining filters, construct a

[nit] It would help to number the 4 cases above and refer to the case numbers in the comments here.

@mgartner mgartner force-pushed the generate-partial-index-scans branch 2 times, most recently from 6aa9099 to deb6c78 Compare June 11, 2020 23:47
Copy link
Collaborator Author

@mgartner mgartner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 2 stale) (waiting on @mgartner and @rytaft)


pkg/sql/opt/testutils/opttester/opt_tester.go, line 830 at r2 (raw file):

Previously, rytaft (Rebecca Taft) wrote…

While you're making changes here, probably worth adding the ruleName to ot.seenRules like we do in Optimize above

I've undid these changes for now, because I no longer need them.


pkg/sql/opt/xform/custom_funcs.go, line 134 at r2 (raw file):

Previously, RaduBerinde wrote…

[nit] which cannot

Done.


pkg/sql/opt/xform/custom_funcs.go, line 191 at r2 (raw file):

Previously, rytaft (Rebecca Taft) wrote…

[nit] It would help to number the 4 cases above and refer to the case numbers in the comments here.

Turns out there's actually 6 cases. I've rewritten this comment to be more clear, and I've rewritten the code to be correct and also less verbose. LMK what you think!


pkg/sql/opt/xform/custom_funcs.go, line 196 at r2 (raw file):

Previously, RaduBerinde wrote…

[nit] Doesn't indexScanBuilder cover this case too?

Thanks for the pointers. I've simplified this code, and fixed a bug—it wasn't splitting the filters into 2 Selects when part of the filters must be applied before the IndexJoin and the remaining filters must be applied after.


pkg/sql/opt/xform/partial_index.go, line 20 at r2 (raw file):

Previously, RaduBerinde wrote…

I think this is accidentally left over

Oops!


pkg/sql/opt/xform/testdata/rules/select, line 223 at r2 (raw file):

Previously, RaduBerinde wrote…

Maybe add a test where the table has multiple partial indexes (as well as some non-partial indexes).

Done.

@mgartner mgartner force-pushed the generate-partial-index-scans branch 2 times, most recently from 0f06df8 to a7f956d Compare June 11, 2020 23:59
Copy link
Collaborator

@rytaft rytaft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 5 of 5 files at r3.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 2 stale)


pkg/sql/opt/xform/custom_funcs.go, line 191 at r2 (raw file):

Previously, mgartner (Marcus Gartner) wrote…

Turns out there's actually 6 cases. I've rewritten this comment to be more clear, and I've rewritten the code to be correct and also less verbose. LMK what you think!

Looks good!

This commit introduces a new exploration rule,
GeneratePartialIndexScans. This rule generates unconstrained Scans over
partial indexes when a Select filter implies the partial index
predicate.

For example, consider the table and query:

    CREATE TABLE t (
      i INT,
      s STRING,
      INDEX pidx (i) STORING (s) WHERE s = 'foo'
    )

    SELECT i FROM t WHERE s = 'foo'

GeneratePartialIndexScans will generate a scan over the partial index:

    project
     ├── columns: i:1
     └── scan t@pidx
          └── columns: i:1 s:3!null

It is capable of handling cases where the index does not cover the
selected columns, and where additional filtering is required after the
partial index scan. For example:

    CREATE TABLE t (
      i INT,
      s STRING,
      f FLOAT,
      INDEX pidx (i) WHERE s = 'foo'
    )

    SELECT i FROM t WHERE s = 'foo' AND f = 2.5

In this case, GeneratePartialIndexScans will generate the following
expression with an added Select and IndexJoin:

    project
     ├── columns: i:1
     └── index-join t
          ├── columns: i:1 s:2!null f:3!null
          └── select
               ├── columns: i:1 rowid:4!null
               ├── scan t@pidx
               │    └── columns: i:1 rowid:4!null
               └── filters
                    └── f:3 = 2.5

Release note: None
@mgartner mgartner force-pushed the generate-partial-index-scans branch from a7f956d to 4702d62 Compare June 15, 2020 16:57
@mgartner
Copy link
Collaborator Author

bors r+

@craig
Copy link
Contributor

craig bot commented Jun 15, 2020

Build failed (retrying...)

@craig
Copy link
Contributor

craig bot commented Jun 15, 2020

Build succeeded

@craig craig bot merged commit 546ec0d into cockroachdb:master Jun 15, 2020
@mgartner mgartner deleted the generate-partial-index-scans branch June 15, 2020 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

opt: generate unconstrained scans over partial indexes
4 participants