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-22.2: opt: backport fix for ordering-related optimizer panics to v22.2 #113175

Merged

Conversation

DrewKimball
Copy link
Collaborator

Backport:

Please see individual PRs for details.

/cc @cockroachdb/release

Release justification: Fix for internal error and possible incorrect results in the optimizer.

It is possible for some functional-dependency information to be visible
to a child operator but invisible to its parent. This could previously
cause panics when a child provided an ordering that could be proven to
satisfy the required ordering with the child FDs, but not with the
parent's FDs.

This patch adds a step to the logic that builds provided orderings that
ensures a provided ordering can be proven to respect the required ordering
without needing additional FD information. This ensures that a parent never
needs to know its child's FDs in order to prove that the provided ordering
is correct. The extra step is a no-op in the common case when the provided
ordering can already be proven to respect the required ordering.

Informs cockroachdb#85393
Informs cockroachdb#87806
Fixes cockroachdb#96288

Release note (bug fix): Fixed a rare internal error in the optimizer that has
existed since before version 22.1, which could occur while enforcing orderings
between SQL operators.
This patch adds a new session setting, `optimizer_use_provided_ordering_fix`,
which toggles whether to use the `finalizeProvided` function introduced in
required ordering. This setting is on by default, and will be used when
backporting cockroachdb#100776.

Informs cockroachdb#113072

Release note: None
@DrewKimball DrewKimball requested a review from a team as a code owner October 26, 2023 21:21
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@DrewKimball
Copy link
Collaborator Author

Are all of the changes protected via a flag or option, new syntax, an environment variable or default-disabled cluster setting?

Yes - there's a default-on setting that can be disabled to disable this change.

What are the risks to backporting this change? Can the risks of backporting be mitigated? What are the risks to not backporting?

If we introduced a bug into the ordering logic, we could cause internal errors and/or incorrect results. This has already baked for months on 23.1 and master, and has an escape hatch, so the risk should be low.

Does this change really need to be backported? Or can it reasonably wait until the next major release to be addressed?

This fixes an incorrect results bug that a customer has hit.

@yuzefovich yuzefovich changed the title opt: backport fix for ordering-related optimizer panics to v22.2 release-22.2: opt: backport fix for ordering-related optimizer panics to v22.2 Oct 27, 2023
Copy link
Collaborator

@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.

:lgtm:

Reviewed 3 of 3 files at r1, 13 of 13 files at r2, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @rafiss)

@yuzefovich
Copy link
Member

Merging since an extraordinary release request has been submitted for this.

@yuzefovich yuzefovich merged commit fcd506d into cockroachdb:release-22.2 Oct 31, 2023
2 checks passed
@DrewKimball DrewKimball deleted the backport22.2-100776-113097 branch October 31, 2023 22:39
@celiala
Copy link
Collaborator

celiala commented Nov 1, 2023

blathers backport staging-v22.2.16

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.

5 participants