release-23.2: inverted: fix intersection of SpanExpressions in some cases

[blathers-crl]

Backport 1/1 commits from #118256 on behalf of @yuzefovich.

/cc @cockroachdb/release

---

This commit fixes the logic for intersecting two SpanExpressions in some cases. In particular, consider the following example:

• left expr is ARRAY['str1'] <@ t.links which translates to a SpanExpression in which only FactoredUnionSpans is set to the span containing str1
• right expr is ARRAY ['str1'] && t.links OR ... which translates to a SpanExpression in which FactoredUnionSpans is set to exactly same span containing str1 plus some other stuff in children expressions.

In other words, we have a AND (a OR b).

When intersecting SpanExpressions, we intersect their FactoredUnionSpans, and then we update FactoredUnionSpans of expressions to subtract the shared ones. Previously, in the example above this would result in making the left expression empty, so it would be pruned. However, that is incorrect - we have the equivalent of left AND right, so we must ensure that left expression is satisfied, and previously this wouldn't be the case. The fix is that if one of the children expressions became empty, then intersection of two children is also empty, so instead of promoting non-empty child we now nil non-empty child out. In the example above, previously we would get a OR b, and now we'll correctly get a.

Fixes: #117979.

Release note (bug fix): Previously, in some cases CockroachDB could incorrectly evaluate queries that scanned an inverted index and had a WHERE filter in which two sides of the AND expression had "similar" expressions (e.g. ARRAY['str1'] <@ col AND (ARRAY['str1'] && col OR ...)), and this is now fixed. The bug has been present since pre-22.2 version.

---

Release justification: bug fix.

[blathers-crl]

Thanks for opening a backport.

Please check the backport criteria before merging:

• Backports should only be created for serious
  issues or test-only changes.
• Backports should not break backwards-compatibility.
• Backports should change as little code as possible.
• Backports should not change on-disk formats or node communication protocols.
• Backports should not add new functionality (except as defined
  here).
• Backports must not add, edit, or otherwise modify cluster versions; or add version gates.
• All backports must be reviewed by the owning areas TL and one additional
  TL. For more information as to how that review should be conducted, please consult the backport
  policy.

If your backport adds new functionality, please ensure that the following additional criteria are satisfied:

• There is a high priority need for the functionality that cannot wait until the next release and is difficult to address in another way.
• The new functionality is additive-only and only runs for clusters which have specifically “opted in” to it (e.g. by a cluster setting).
• New code is protected by a conditional check that is trivial to verify and ensures that it only runs for opt-in clusters. State changes must be further protected such that nodes running old binaries will not be negatively impacted by the new state (with a mixed version test added).
• The PM and TL on the team that owns the changed code have signed off that the change obeys the above rules.
• Your backport must be accompanied by a post to the appropriate Slack
  channel (#db-backports-point-releases or #db-backports-XX-X-release) for awareness and discussion.

Also, please add a brief release justification to the body of your PR to justify this
backport.

[cockroach-teamcity]

This change is 

[rytaft]

Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @mgartner, @rafiss, and @yuzefovich)

[mgartner]

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @blathers-crl[bot], @rafiss, and @yuzefovich)

---

pkg/sql/inverted/testdata/expression line 645 at r1 (raw file):

----
span expression
 ├── tight: true, unique: false

Why is this tight if we don't have spans for b AND c?

[rytaft]

pkg/sql/inverted/testdata/expression line 645 at r1 (raw file):

Previously, mgartner (Marcus Gartner) wrote…

Why is this tight if we don't have spans for b AND c?

Tight means that the data returned by the span expression exactly matches the data that should be returned by the SQL expression, and therefore we don't need an additional filter.

[mgartner]

pkg/sql/inverted/testdata/expression line 645 at r1 (raw file):

Previously, rytaft (Rebecca Taft) wrote…

Tight means that the data returned by the span expression exactly matches the data that should be returned by the SQL expression, and therefore we don't need an additional filter.

My confusion is how do the spans from only a fully describe a OR (b AND c)?

[mgartner]

Reviewable status: complete! 2 of 0 LGTMs obtained (waiting on @rafiss and @yuzefovich)

---

pkg/sql/inverted/testdata/expression line 645 at r1 (raw file):

Previously, mgartner (Marcus Gartner) wrote…

My confusion is how do the spans from only a fully describe a OR (b AND c)?

Oh. a AND (a OR (b AND c)) is just a. I misread the comment above: