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: always add deletable index predicates to table metadata #58714

Merged

Conversation

mgartner
Copy link
Collaborator

There is no need to avoid adding deletable indexes to the table metadata
in the context of a Select. Exploration rules that require access to
partial index predicates must already discriminate between public and
write/delete-only indexes. For example, scanIndexIter only considers
public indexes. Therefore, the partial index predicates in the table
metadata do not need to be an indication of what indexes are available
for a Select query to use. This reduces the complexity of some
optbuilder code.

Release note: None

@mgartner mgartner requested a review from a team as a code owner January 11, 2021 18:04
@cockroach-teamcity
Copy link
Member

This change is Reviewable

There is no need to avoid adding deletable indexes to the table metadata
in the context of a Select. Exploration rules that require access to
partial index predicates must already discriminate between public and
write/delete-only indexes. For example, `scanIndexIter` only considers
public indexes. Therefore, the partial index predicates in the table
metadata do not need to be an indication of what indexes are available
for a Select query to use. This reduces the complexity of some
optbuilder code.

Release note: None
@mgartner mgartner force-pushed the always-add-deletable-indexes-to-metadata branch from fec2ea2 to a308dd5 Compare January 11, 2021 18:48
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)

@mgartner
Copy link
Collaborator Author

bors r=RaduBerinde

@craig
Copy link
Contributor

craig bot commented Jan 11, 2021

Build succeeded:

@craig craig bot merged commit 4dd430a into cockroachdb:master Jan 11, 2021
@mgartner mgartner deleted the always-add-deletable-indexes-to-metadata branch January 12, 2021 01:16
mgartner added a commit to mgartner/cockroach that referenced this pull request Aug 14, 2021
This commit removes a stale comment that should have been removed in
a previous commit (see cockroachdb#58714).

Release note: None
craig bot pushed a commit that referenced this pull request Aug 14, 2021
68904: dev: strip out trailing slashes in targets/packages r=rail a=rickystewart

Bash auto-complete often puts these trailing slashes in, but we can
strip them out before we pass them on to `bazel` (which will throw an
error if package names end with a slash).

Release note: None

68905: colexec: fix recent change to the parallel unordered synchronizer r=yuzefovich a=yuzefovich

We recently made the parallel unordered synchronizer cancel its local
inputs eagerly whenever the sync transitions into the draining state.
However, we currently incorrectly calculate whether the input is
"local": essentially we look only at the children of each input tree
root but not deeper.

In order to fix it we could have implemented a proper input tree
traversal, but it has its complications because we sometimes wrap
operators with another, and in order to perform the type check, we would
need to unwrap the operator first. That could be error-prone.

Instead, this commit chooses a more safe option of using the eager
cancellation only if the plan is fully local - in this case we know for
sure that aren't any inboxes. This is still sufficient for the use case
of parallel TableReaders in the local flows that this eager cancellation
was introduced for.

Fixes: #68884.

Release note: None (no stable release with this bug)

68919: logictest: move window functions from aggregate file r=yuzefovich a=yuzefovich

Release note: None

68927: opt: remove stale comment r=mgartner a=mgartner

This commit removes a stale comment that should have been removed in
a previous commit (see #58714).

Release note: None

Co-authored-by: Ricky Stewart <ricky@cockroachlabs.com>
Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com>
Co-authored-by: Marcus Gartner <marcus@cockroachlabs.com>
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