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

release-20.2: opt: build all partial index predicate expressions in TableMeta #58274

Merged
merged 2 commits into from
Dec 29, 2020

Conversation

mgartner
Copy link
Collaborator

Backport 2/2 commits from #57707. The backport #58272 relies on these changes.

/cc @cockroachdb/release


opt: build all partial index predicate expressions in TableMeta

Previously, partial index predicates were only built and added to
TableMeta.PartialIndexPredicates if the scope of the scan in
Builder.buildScan included all ordinary table columns. This behavior
prevented errors when building partial index predicate expressions for
foriegn key check scans. These scans do not include all columns, so when
building a predicate expression that referenced a table column not
included, optbuilder would err with "column does not exist".

This commit changes this behavior so that partial index predicates are
always built and added to TableMeta.PartialIndexPredicates. In order
to do this, Builder.addPartialIndexPredicatesForTable creates its own
table scope. It also constructs a new scan rather than relying on the
already-built scan. Constructing a scan is required for considering
logical properties while normalizing partial index predicates.

See discussion at #57622
for more context on this change.

Release note: None

optbuilder: reduce redundant building of arbiter filter expressions

Previously, the arbiter predicate provided in the WHERE clause of an
INSERT ... ON CONFLICT statement was rebuilt as a memo.FiltersExpr
repetitively while the optbuilder determined arbiter indexes. Now, the
expression is built only once, reducing work.

Release note: None

@mgartner mgartner requested a review from RaduBerinde December 24, 2020 19:34
@mgartner mgartner requested a review from a team as a code owner December 24, 2020 19:34
@cockroach-teamcity
Copy link
Member

This change is Reviewable

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:

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

Previously, partial index predicates were only built and added to
`TableMeta.PartialIndexPredicates` if the scope of the scan in
`Builder.buildScan` included all ordinary table columns. This behavior
prevented errors when building partial index predicate expressions for
foriegn key check scans. These scans do not include all columns, so when
building a predicate expression that referenced a table column not
included, optbuilder would err with "column does not exist".

This commit changes this behavior so that partial index predicates are
always built and added to `TableMeta.PartialIndexPredicates`. In order
to do this, `Builder.addPartialIndexPredicatesForTable` creates its own
table scope. It also constructs a new scan if the provided scan does not
output all column in the table scope. A scan on the table and its
logical properties are required in order to fully normalize partial
index predicates.

See discussion at cockroachdb#57622
for more context on this change.

Release note: None
Previously, the arbiter predicate provided in the `WHERE` clause of an
`INSERT ... ON CONFLICT` statement was rebuilt as a `memo.FiltersExpr`
repetitively while the optbuilder determined arbiter indexes. Now, the
expression is built only once, reducing work.

Release note: None
@mgartner mgartner merged commit c6dadb7 into cockroachdb:release-20.2 Dec 29, 2020
@mgartner mgartner deleted the backport20.2-57707 branch December 29, 2020 00:47
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.

3 participants