fix(plan): preserve NULL semantics for index upper bounds - #26848
Conversation
|
Evidence for cdd2583: focused nullable strict-< planner tests PASS; full pkg/sql/plan PASS; pkg/sql/plan build/vet PASS; controlled mo-service build PASS; complete diff self-review PASS against origin/main 45a917d. Baseline public-path witness was automatic 1,3,4 versus PRIMARY 3,4. No CI monitoring requested. |
|
Independent triple-candidate acceptance on 10.222.1.55 passed Round 32 at full scale: 80,000 rows, 24 deterministic UPDATE epochs, 8 index layouts, 10 query families, all default/PRIMARY/forced routes, and periodic hidden-table decoded-key audits; failures=0 versus 100 baseline strict-upper failures. After restarting the same candidate, read-only replay again reported failures=0 and revalidated all 8 hidden tables by count, distinct PK, and every indexed column via |
|
Additional Round 31 triple-candidate acceptance: 124,971 rows after nullable/boundary UPDATE, DELETE/INSERT and flush. Direct vs expression oracle pairs all match: numeric 103/SUM 5727235, text 4474/251691893, NULL 584/35054467, PK range 44014/2420806315. All four hidden indexes report 124971 rows / 124971 distinct PK; decoded current-value mismatch is 0 for every indexed component, including nullable BIGINT/VARCHAR, DECIMAL and explicit PK positions. |
|
Triple integration review found that #26850 candidate cost scoring could demote #26848 forced covering access to a backfill join after nullable-residual cost was included. The integration-only finding is fixed in #26850 head 0278375 with a self-contained covering-vs-backfill FORCE regression independent of #26848. The combined conflict resolution still preserves #26848 nullable strict-< decoded residual trigger together with #26850 deduplicated residual rechecks at Selectivity=1. Exact triple heads: #26848 b4c2d1b, #26850 0278375, #26851 18091e7. All 38 added planner tests and full pkg/sql/plan passed; controlled build/vet and make build passed. No remaining incompatibility found. |
|
Five-PR live combination acceptance passed on 55:
Important interaction found during staging: the four-way candidate without #26806 made the base/shadow DECIMAL path correct via #26815 while the secondary-index range path remained wrong ( |
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
## Summary - let forced ORDER BY access skip a contiguous leading index prefix fixed by single-value equality - preserve plain `FORCE INDEX` as an access-path constraint when the named index cannot provide ordering, leaving `Sort` to enforce order - keep `FORCE INDEX FOR ORDER BY` order-scoped and leave ordinary optimization unchanged ## Root cause The force-index ORDER BY prepass required ORDER columns to match the physical index from part zero. For an index such as `(status, amount, id)`, it therefore rejected a fixed status prefix followed by `ORDER BY amount, id` even though that prefix is order-neutral. When the order match failed, base-scan protection also caused an unscoped plain `FORCE INDEX` to be silently discarded. The matcher now skips only a contiguous prefix fixed by `column = runtime_constant`. IN, range, and unconstrained or gapped parts remain order-incompatible. If an unscoped plain force cannot satisfy order, the planner still builds the named covering scan or index backfill join without marking it ordered, so the existing Sort remains authoritative. ## Validation Fresh validation from isolated worktree `/home/xupeng/github/matrixone-issue-26814`, based on `origin/main` `0676200da47052659cf46ad9e22d67ab9654e393`: - regression reproduced before the production change: equality-prefix and plain-force fallback cases failed to retain the index - focused force/order planner tests: pass - full `pkg/sql/plan` suite: pass - package `go build` and `go vet` with repository-local CGo paths: pass - whole `make build`: pass - isolated candidate-binary BVT `optimizer/index_hint.sql`: 53/53 statements pass - pairwise merge plus focused/full planner suites with draft #26848: pass - pairwise merge plus focused/full planner suites with draft #26850 (for #26803): pass The regression matrix covers ASC/DESC, LIMIT/no LIMIT, equality/IN/range/gapped leading parts, covering/backfill access, invalid hints, unhinted optimization, and exact/distinct row checks. No row-path benchmark was added: the new work is planner-only and bounded by index parts times scan filters plus ORDER columns. Fixes #26814
## Summary - compare all automatic regular secondary-index access candidates in one encoded-byte-work cost domain - cover both index-only scans and non-covering INDEX joins, including extraction consumers, residual predicates, compound-PK extraction, and base-row backfill - rank scorable candidates independently of catalog order; use an unscorable fallback only when every matching candidate is unscorable - preserve selective-index wins, explicit FORCE INDEX behavior, small-table compatibility, spatial-index priority, and safe LIMIT/OFFSET handling ## Root cause Regular-index planning selected the first usable covering or backfill shape and did not compare encoded key width, candidate cardinality, repeated `serial_extract` consumers, hidden residual work, and base-table rechecks across sibling candidates. Rejecting only an index-only shape could therefore move a query to an even more expensive INDEX join, while catalog order could hide a profitable narrow sibling. The fix builds one query-scoped cost context, scores covering/point/range/backfill alternatives, and mutates the plan only after selecting the global winner. Invalid statistics or malformed candidate metadata fail open without allowing an unscorable candidate to beat any scorable candidate. ## Validation Fresh local validation on Linux/amd64 with Go 1.26.4 and repository-local CGo artifacts: - `GOWORK=off go list -mod=readonly ./pkg/sql/plan` - `GOWORK=off go build -mod=readonly ./pkg/sql/plan` - `GOWORK=off go vet -mod=readonly ./pkg/sql/plan` - 30-test focused encoded-index planner selection: pass - full `pkg/sql/plan` suite: pass (`-count=1 -timeout=120s`) - `BenchmarkEncodedIndexCostWideTableMultiIndex`, 5 runs: 20.125–20.546 µs/op, 14,680 B/op, 49 allocs/op Accepted service evidence on the 120k-row workload: - 9-repeat default/base-scan p50 11.074 ms versus FORCE 16.544 ms and scan control 11.709 ms - restart p50 11.5665 ms - all four hidden tables audited at 120000/120000 rows - prepared DDL ordering reproduced the old index remaining selected until it was dropped No shared service switch was performed during #26847 validation. ## Compatibility and interaction - automatic plans fail open when every matching candidate is unscorable - valid candidates always outrank unscorable candidates, in either catalog order - equal valid scores retain catalog order - dynamic or overflowing LIMIT/OFFSET and residual-filter pagination are not capped - repeated physical parts retain duplicate lookup arguments while selectivity and residual work are charged once #26848 has one manual merge conflict in `pkg/sql/plan/apply_indices.go`: its nullable strict-upper-bound trigger overlaps this PR's deduplicated, `Selectivity=1` residual construction. Its tests merge automatically. The combined resolution should retain #26848's trigger and this PR's one-copy/non-selective residual accounting. Fixes #26803
2 participants
Summary
Root cause
A serialized nonunique index key orders encoded NULL below an encoded non-NULL bound. Rewriting
nullable_col < bounddirectly onto that key and removing the original predicate therefore admitted rows for which SQL comparison evaluates to unknown.The fix keeps the serialized comparison as the access predicate and rechecks the decoded indexed value only for nullable canonical
<comparisons. Non-nullable comparisons keep the existing single-filter path.Validation
1,3,4; PRIMARY oracle3,4pkg/sql/plantest suite passedgo buildandgo vetforpkg/sql/planpassed with repository-local CGo pathsmake buildproduced amo-servicecandidateorigin/main45a917d7f8Fixes #26821