opt: preserve NULL partition boundaries in scan constraints - #174767
Open
Alignyx wants to merge 1 commit into
Open
opt: preserve NULL partition boundaries in scan constraints#174767Alignyx wants to merge 1 commit into
Alignyx wants to merge 1 commit into
Conversation
Partition-derived optional filters must cover both defined partition keys and every gap between them. SQL tuple comparisons against a boundary that contains NULL do not provide that coverage: equality cannot be true, and strict inequalities exclude rows matching the prefix before NULL. The optimizer can consequently omit entire prefixes from scans and mutations. Expand comparisons for NULL-containing partition boundaries using null-safe equal prefixes and lexicographic comparisons that treat NULL as the smallest key value. Keep the existing tuple path for non-NULL boundaries and leave ordinary SQL comparison semantics, partition metadata and storage unchanged. Add native key-coverage and SQL regressions for rows inside and outside the defined partitions, predicates on later index columns, and limited updates. Retain historical first-column NULL and non-NULL partition regressions. Resolves: cockroachdb#137994 Epic: none Release note (bug fix): Fixed an optimizer bug that could omit qualifying rows from SELECT and UPDATE statements on tables with list-partitioned indexes whose partition boundaries contain NULL values. This could occur with or without a NO_FULL_SCAN hint.
|
Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR. My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
This file contains hidden or 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
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.
Summary and scope
Fix missing rows in partition-derived index scans when a LIST partition boundary
contains NULL. The optimizer must cover both defined partition keys and every gap
between them. Ordinary SQL tuple comparisons against NULL do not express that
physical key coverage, so the existing optional filters can omit entire leading
prefixes before filtering, ordering, limiting, or updating rows.
This PR corrects that metadata-to-filter translation in
columnComparison, usingnull-safe equal prefixes and lexicographic comparison for NULL-containing
boundaries. It preserves the existing non-NULL boundary path, ordinary SQL
comparison semantics, partition definitions, key encoding, and index layout.
The fix addresses #137994's NULL-boundary failure, including unhinted plans. It
does not disable partition-derived scans or reject existing legal partitions.
Problem and correct-result oracle
The investigation retained the report's types, predicate, ordering and limit:
All eight explicit names satisfy this predicate, and the limit exceeds the row
count. The correct result bag therefore contains all eight rows, independently
of which index is chosen. Likewise, the corresponding UPDATE must affect all
eight rows. The baseline instead selects/updates only
danddd.The omitted rows include both defined partition tuples with a NULL second column
and rows with the same leading values but a non-NULL second column. These are
valid stored rows; partition metadata must not be treated as an exhaustive
restriction on possible data.
Metamorphic relations used in the investigation
MR1: access-path invariance
Keep the schema, data, SQL types, predicate, projection, ordering and limit fixed;
change only the access path. The equivalent control uses
tab@tab_pkeyin placeof
tab@{NO_FULL_SCAN}:The primary-index control returns the independently expected eight rows. The
hint is not itself a correctness oracle; the explicit input set establishes the
expectation and the different plan helps locate the failure.
Importantly, simply removing
NO_FULL_SCANis not a correct control on thetested baseline. Its unhinted SELECT and UPDATE also return or update only
dand
dd. Those were classified as additional failing W cases, not used to definethe expected result. This also agrees with the issue's
unhinted reduced reproduction.
MR2: materialize the complete input before applying the same query
For this pure query, introducing an optimization boundary around the complete
primary-index input preserves the result. It changes where partition-derived
constraints can participate, providing another eight-row control without changing
the predicate or widening its types. This relation is not an unconditional claim
about volatile expressions or evaluation-order-dependent errors.
MR3: selected target rows and rollback-isolated UPDATE effects agree
The target set selected with the same predicate, ordering and limit must match
the rows changed by the UPDATE. Each mutation probe ran in its own transaction:
The same probe was repeated unhinted and with a forced-primary UPDATE. Rolling
back restores the initial data for each comparison; subsequent checks
require eight rows and zero
barmarkers. The primary scan used to inspect theeffects avoids reusing the faulty access path as the observer.
The main
LIMIT 84cannot truncate eight rows, so ties inORDER BY col1_3donot make the expected target bag ambiguous. Held-out tests that actually truncate
results use a unique ID tie-breaker and explicit NULL ordering to define a
deterministic selected set. The investigation does not assume arbitrary tied
LIMIT queries must choose the same rows under different plans.
Root cause and how it was localized
GenerateConstrainedScansuses optional filters derived from both partitionvalues and the intervals between them.
inPartitionFiltersandinBetweenFilterssharecolumnComparison. Their resulting constraints mustjointly cover every possible key before the query's actual predicates are applied.
For a boundary
(1,NULL), ordinary SQL comparisons are unsuitable for that job:(a,b) = (1,NULL)cannot be TRUE, including for the boundary tuple itself.(a,b) > (1,NULL)admitsa > 1, but not rows witha = 1and a non-NULLb.entire
a = 1prefix rather than just an impossible SQL predicate result.idxconstraintis correctly respecting SQL three-valued comparison semantics.The invalid step is expressing partition-key boundaries as those SQL tuple
predicates. The correction belongs at that translation boundary, not in the
general SQL comparator or constraint engine.
EXPLAIN (OPT, VERBOSE)exposed the missing prefixes, whileEXPLAIN ANALYZEshowed that rows were lost at the scan, before filtering or mutation. The main
baseline/candidate comparison is:
d, ddd, dddistsql=alwaysSELECT controlsd, ddThree complete baseline and three complete candidate matrices agree. In the
candidate, W retains the secondary-index scan, filter, top-k and index join.
The important lower span bounds change as follows:
/2/1/NULL/1000000001/1000000000/NULL/2000000001/2000000000/NULLThe candidate now reads eight secondary entries and performs eight primary
lookups; the full-primary control reads eight KV rows. This is corrected scan
coverage, not a fallback that avoids the original partitioned path.
The main plan observations are local, single-node execution. Its
distsql = alwaysarm is a distribution-setting control, not proof of remote-nodeexecution. These observations plus source analysis are not instrumented
branch-coverage traces.
How the repair works
Only a boundary containing NULL takes the new path. Comparisons use the existing
logical datum ordering, where NULL is below non-NULL values. Index ASC/DESC
directions are still interpreted by
idxconstraint; this does not assertthat NULL has the smallest encoded byte position in every descending index.
For equality, the expansion requires null-safe equality at every boundary
position, using
ConstructIs. For strict comparisons, it builds the expressionfrom the last position toward the first:
The strict-comparison suffix starts as FALSE; the equality suffix starts as TRUE.
The per-position rules are:
column IS NOT NULLcolumn < v OR column IS NULLcolumn > vFor
(a,b)versus(1,NULL), this retains equality at(1,NULL)and admitsa = 1, b IS NOT NULLinto the greater-than side. Thus both the defined partitionand the gap immediately after it remain represented.
The required invariant concerns accepted TRUE membership. An expression can
still evaluate to UNKNOWN outside that set; the patch does not redefine SQL's
three-valued logic. Generated index constraints may safely overapproximate the
predicate, but must not omit a matching key. Residual query filters remain in
place to enforce the original query.
This shared helper repairs both partition and in-between filters. Boundaries
without NULL keep the existing tuple path, and the historical first-column
IS NULLcompensation is retained. No tuple elements are reordered, partitiondefinitions rejected, or storage structures changed.
Discussion, historical precedent, and existing fixtures
The root-cause discussion
identified fragile NULL tuple span generation and considered restricting NULL in
partition tuples. This PR takes the narrower correctness approach of representing
the existing legal boundaries correctly. It does not claim the entire partition
constraint framework has been redesigned.
Historical commit a15bda7,
from PR #63932 fixing #63733, added first-column NULL coverage and regressions.
That compensation and its positive controls are preserved here.
The full optimizer suite exposed three old expected plans whose spans needed
audited corrections, not wholesale snapshot acceptance:
limitplans now start at the missing/key/NULLboundaries, includingthe original report's leading values and another set of
1,5,5000000.Their predicates, ordering,
LIMIT 84, and positive/negativeSplitLimitedSelectIntoUnionSelectsassertions are unchanged.t63733_nullsplan now has valid constrained spans where itpreviously scanned without a constraint. Its required
c = 1filter remains;coverage review confirms that only nonqualifying keys are excluded.
These are ten span-content changes across two files and three plans. A positive
rule assertion establishes application during exploration, not that the final
chosen plan must contain a Union operator.
Completed local validation
Results refer to existing commit
f7581d42fd0129f315fa5e4ed3b9918bd99887cf, basedon
8812064a015d2faf99d3fc7e15880f94042954b0. They describe completed localvalidation, not upstream PR CI status.
TestPartitionBoundary137994has 600 directcomparison cases: 25 NULL-containing boundary tuples, three comparisons and
eight ASC/DESC combinations. Each enumerates 27 keys. An independent
Datums.Compareoracle checks accepted TRUE membership and matching-key spancoverage. Another 48 cases check the combined partition/complement coverage
for duplicates, prefixes, nested NULLs, gaps, leading NULL and non-NULL controls.
The frozen test fails semantically on baseline and passes on the candidate.
non-NULL suffixes and leading NULL values outside the defined partitions.
Checks include the limited scan, a later-column predicate, UPDATE effects and
rollback preservation. The baseline stops at the first SELECT, returning four
rows instead of eight; later statements are not claimed to have independent
native-red observations. The complete unchanged regression passes with the fix.
baseline/candidate outcome checks, including baseline span bounds, observed
scan/root counts and rollback.
Fifteen deliberately corrupted or mislabeled evidence variants are rejected.
the candidate, correcting eight baseline W deviations. Cases cover NULL in all
three positions, shared/DEFAULT prefixes, descending indexes, deterministic
limited selections and UPDATE effects. Six tables retain all 94 rows with zero
rollback markers. Three hand-computed LIMIT checks and eleven verifier rejection
self-tests pass.
norm, xform, props and execbuilder unit targets pass. On final verification,
six unchanged successful targets are cached; full xform reruns after the audited
expected-span updates. Eleven named partition/index logic executions pass across
supported local, fakedist and 5node configurations. Six complete execution-plan
suites (
select,select_index,update,delete,limit,topk) and all16 SQL test shards also pass.
The committed scope is six files (+291/-9): the 42-line production diff, a
172-line unit regression, a 68-line SQL regression, two Bazel entries and the
audited changes to two historical plan fixtures. Supplemental replay and held-out
artifacts are retained in the local investigation record, not included as extra
PR files.
Current limitations
dependence on current span-generation behavior remain. This is not a complete
rewrite or proof of every partition-derived optimization.
possible spans. Existing mixed-direction limitations tracked by opt: support constrained in between filters with multiple partitioning columns #81456 remain;
eight-direction unit coverage does not establish that all mixed ASC/DESC
partition scans now produce useful or optimal constraints.
index alone is not a rule-execution trace, and enabling vectorization does not
prove UPDATE operators themselves ran in a vectorized engine. Result checks
establish bags or deterministic selected sets, not every original output's
natural row order.
mixed-version upgrade matrix or all-release backport certification is supplied.
NULL-containing boundaries now build linear-size lexicographic expressions;
their wider-workload planning costs have not been quantified.
executions; it cannot retroactively complete application UPDATE statements
that already missed rows. Any reconciliation of historical application effects
is separate from this optimizer change.
Resolves: #137994
Epic: none
Release note (bug fix): Fixed an optimizer bug that could omit qualifying
rows from SELECT and UPDATE statements on tables with list-partitioned
indexes whose partition boundaries contain NULL values. This could occur
with or without a NO_FULL_SCAN hint.