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

indexrec: improve ordering recommendations #73451

Closed
nehageorge opened this issue Dec 3, 2021 · 0 comments · Fixed by #73565
Closed

indexrec: improve ordering recommendations #73451

nehageorge opened this issue Dec 3, 2021 · 0 comments · Fixed by #73565
Assignees
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)

Comments

@nehageorge
Copy link

Previously, in order to avoid redundant recommendations, we standardized the index candidate creation for *memo.SortExprs, or ordering candidates. More specifically, if the recommendation query contained ORDER BY k DESC, i ASC we would create an index candidate with the key (k, i DESC), allowing for a reverse scan of the index. We would also create the same candidate for ORDER BY k ASC, i DESC, allowing for a forward scan of the index.

Since reverse scans are less efficient than forward scans, this is not ideal. We should make the best recommendation (i.e. forward scans where possible), while still avoiding redundant recommendations.

@nehageorge nehageorge added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Dec 3, 2021
@nehageorge nehageorge self-assigned this Dec 3, 2021
nehageorge pushed a commit to nehageorge/cockroach that referenced this issue Dec 7, 2021
Previously, in order to avoid redundant recommendations, we standardized the
index candidate creation for *memo.SortExprs, or ordering candidates. More
specifically, if the recommendation query contained `ORDER BY k DESC, i ASC`
we would create an index candidate with the key (k, i DESC), allowing for a
reverse scan of the index. Since reverse scans are less efficient than
forward scans, this is not ideal.

In this commit, we no longer standardize ordering recommendations. We always
create the sort candidate according to how it is in the query.

There is no handling for redundant recommendations (meaning recommending an
index on `(k ASC)` and `(k DESC)`). Since queries usually only have one
`ORDER BY` clause, it doesn't seem like an important case to consider.
Plus, through experimentation, it seems that when there are redundant
candidates, (only possible for single column indexes), there is no redundant
recommendation. See tests added in this PR.

Fixes: cockroachdb#73451.

Release note: None
nehageorge pushed a commit to nehageorge/cockroach that referenced this issue Dec 14, 2021
Previously, in order to avoid redundant recommendations, we standardized the
index candidate creation for *memo.SortExprs, or ordering candidates. More
specifically, if the recommendation query contained ORDER BY k DESC, i ASC
we would create an index candidate with the key (k, i DESC), allowing for a
reverse scan of the index. Since reverse scans are less efficient than
forward scans, this is not ideal.

In this commit, we no longer standardize ordering recommendations. We always
create the sort candidate according to how it is in the query.

There is no handling for redundant recommendations (meaning recommending an
index on (k ASC) and (k DESC)). Since queries usually only have one
ORDER BY clause, it doesn't seem like an important case to consider.
Plus, through experimentation, it seems that when there are redundant
candidates, (only possible for single column indexes), there is no redundant
recommendation. See tests added in this PR.

Fixes: cockroachdb#73451.

Release note: None
craig bot pushed a commit that referenced this issue Dec 16, 2021
73565: indexrec: improve ordering recommendations r=nehageorge a=nehageorge

Previously, in order to avoid redundant recommendations, we standardized the
index candidate creation for *memo.SortExprs, or ordering candidates. More
specifically, if the recommendation query contained `ORDER BY k DESC, i ASC`
we would create an index candidate with the key (k, i DESC), allowing for a
reverse scan of the index. Since reverse scans are less efficient than
forward scans, this is not ideal.

In this commit, we no longer standardize ordering recommendations. We always
create the sort candidate according to how it is in the query.

There is no handling for redundant recommendations (meaning recommending an
index on `(k ASC)` and `(k DESC)`). Since queries usually only have one
`ORDER BY` clause, it doesn't seem like an important case to consider.
Plus, through experimentation, it seems that when there are redundant
candidates, (only possible for single column indexes), there is no redundant
recommendation. See tests added in this PR.

Fixes: #73451.

Release note: None

Co-authored-by: Neha George <neha.george@cockroachlabs.com>
@craig craig bot closed this as completed in ab1a6cd Dec 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant