release-22.2: merge tag v22.2.16 into release-22.2 #113917
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
Previously, we had the following bug in how the streamer was used under the following conditions: - we're looking up into a table with at least 3 column families - not all column families are needed, so we end up creating "family-specific" spans (either Gets or Scans - the latter is possible when multiple contiguous families are needed) - the streamer is used with OutOfOrder mode (which is the case for index joins when `MaintainOrdering` is `false` and for lookup joins when `MaintainLookupOrdering` is `false`) - the index we're looking up into is split across multiple ranges. In such a scenario we could end up with KVs from different SQL rows being intertwined because the streamer could execute a separate BatchRequest for those rows, and in case `TargetBytes` limit estimate was insufficient, we'd end up creating "resume" batches, at which point the "results" stream would be incorrect. Later, at the SQL fetcher level this could either manifest as an internal "non-nullable column with no value" error or with silent incorrect output (we'd create multiple output SQL rows from a true single one). This problem is now fixed by asking the streamer to maintain the ordering of the requests whenever we have `SplitFamilyIDs` with more than one family, meaning that we might end up creating family-specific spans. Note that the non-streamer code path doesn't suffer from this problem because there we only parallelize BatchRequests when neither `TargetBytes` nor `MaxSpanRequestKeys` limits are set, which is the requirement for having "resume" batches. Release note (bug fix): CockroachDB previously could incorrectly evaluate lookup and index joins into tables with at least 3 column families in some cases (either "non-nullable column with no value" internal error would occur or the query would return incorrect results), and this is now fixed. The bug was introduced in 22.2.
This commit adds `streamer_always_maintain_ordering` session variable that - when set to `true` - forces all current usages of the streamer in SQL layer (lookup and index joins) to ask it to maintain the ordering, even if this is not stricly necessary by the query. This variable is introduced as a possible workaround in case we find more scenarios where we currently are incorrectly using the OutOfOrder mode of the streamer. Release note: None
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
…ort-staging-v22.2.16-113175
…ort-staging-v22.2.16-113109 staging-v22.2.16: release-22.2: sql: fix usage of streamer with multiple column families (cockroachdb#113694) Co-Authored-By: Yahor Yuzefovich <[email protected]>
Epic: none Release note: none Release justification: none
![blathers[crl]](https://avatars.githubusercontent.com/in/59700?s=40&u=373d9101a12edc3847d2fb6ae78e3adcfb4ca41e&v=4){kind=avatar}
|
Thanks for opening a backport. Please check the backport criteria before merging:
If your backport adds new functionality, please ensure that the following additional criteria are satisfied:
Also, please add a brief release justification to the body of your PR to justify this |
blathers-crl
bot
added
the
backport
|
This change is |
|
I'm not totally sure this is necessary, but I stole a step from the 23.1+ phase 3 runbook and did this anyway for 22.2. It seemed weird not to. |
rail
approved these changes
Nov 7, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
v22.2.16 was tagged from a sub-branch off of release-22.2; this merges it back into the release branch so it appears in the linage of subsequent commits.
Epic: none
Release note: none
Release justification: none