[cpegeric]

What type of PR is this?

• API-change
• BUG
• Improvement
• Documentation
• Feature
• Test and CI
• Code Refactoring

Which issue(s) this PR fixes:

issue #31486

Related to #30402 and #31119. This PR does not fix #31119: the PMC term request orders by doc_id ASC first, which is still not bounded.

What this PR does / why we need it:

ORDER BY MATCH(...) DESC LIMIT k already bounds the FULLTEXT2 search to the top LIMIT+OFFSET rows. Adding the primary key as a tie-break, as in ORDER BY score DESC, id LIMIT k, turned the bound off. The search then returned every match to a full sort, so the query's cost grew with the number of matches, not with k. This is the usual shape for a stable result page.

With this PR, the bound applies whenever the first ORDER BY key is the MATCH score DESC (default null placement) and the query has a LIMIT. The existing sort above the search applies the later keys to the bounded rows, along with ORDER BY, LIMIT and OFFSET.

This applies to both paths that bound the score-only form: the covered search (primary key and INCLUDE columns only) and the base-table join added in #30618. Every other guard is unchanged: a non-score first key, ascending score, SQL_CALC_FOUND_ROWS, a residual filter, a user join or an aggregate still decline.

Semantics (developer decision)

Decided by @cpegeric, 2026-10-27. This supersedes the 2026-10-23 decision in PR #30618's review; the design doc's decision record is updated.

Only rows tied at the k-th score are affected. The search keeps the top LIMIT+OFFSET rows by score. When rows tie at the cutoff, the search picks which of them are kept, exactly as it already does for ORDER BY score DESC LIMIT k. The primary key orders the returned rows; it does not choose among the tied rows. Every returned row scores at least as high as every row left out, and rows above the cutoff score are always returned.

Why this is accepted:

• Relevance scores are FLOAT32 BM25 values. Rows tie at the cutoff only with identical term statistics (for example duplicate documents).
• Exact tie resolution would mean carrying the key into the search, or keeping every tied row through each segment, worker and merge. Both were rejected on 2026-10-23: vector indexes also bound their top-k by a single key.
• The alternative is the current unbounded plan, which every query of this shape pays for.

Results

Warm same-data A/B: 300,000 documents, a term in every document, FULLTEXT2 index with INCLUDE (covered path), one client, 20 runs after 3 warm-up runs. Single-CN local build: main 1fd12d526e vs this PR.

Query	main p50	PR p50	Rows the search returns
ORDER BY score DESC, id LIMIT 10	23.94 ms	14.19 ms	300,000 → 10
ORDER BY score DESC, id LIMIT 1000	26.72 ms	16.52 ms	300,000 → 1,000
ORDER BY score DESC LIMIT 10 (already bounded on main)	14.11 ms	13.97 ms	10 → 10

With the change, the tie-break form costs the same as the score-only form. Before, its cost grew with the number of matches.

Tests

• Planner unit tests: a secondary key after score DESC is bounded on the covered path (typed fixture, bound SQL, with and without the implicit score sort) and on the base-table join path. id, score DESC and a non-score first key still decline. The bounded cases fail without the change.
• BVT fulltext2_candidate_limit:
  • EXPLAIN shows Candidate Limit for the secondary-key forms on the covered and join paths, and none for a non-score first key.
  • The tied-cutoff cases (ft_tie) assert only answers that hold whichever tied rows the search keeps.
• fulltext2_threads_build uses limit 5, which ends on a score boundary, so the serial and pipelined builds are still compared exactly.
• fulltext2_include_recheck: inside a transaction with deleted rows, the secondary-key form now gets the same short page as the score-only form. That lag behaviour is the existing [Bug]: FULLTEXT2 index lags the base table (ISCP latency): deleted, stale or too few rows until it syncs #30853 decision.
• pkg/sql/plan unit tests pass. BVT: fulltext2 1341/1341, pessimistic_transaction/fulltext2 930/930.

🤖 Generated with Claude Code

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[XuPeng-SH]

mo-bot review · mo-bot-local · done

Commit: 235055de2a0621c2b8b4c5166f911a3bf816d771

Review completed. Read the assessment.

[XuPeng-SH]

mo-bot review · mo-bot-local · review

Commit: 235055de2a0621c2b8b4c5166f911a3bf816d771

Assessment: approve

Reviewed the complete nine-file delta. The implementation matches #31486’s revised cutoff-tie contract, which supersedes earlier secondary-key findings. Short pages during index lag remain accepted under #30853. No actionable unresolved findings.

Model: gpt-6-astra / xhigh

Session: 01a13dcf-1586-7a01-9000-d45e20995cf8 · slot 1 · generation 1

Validation:

• Traced covered and base-join planning through candidate limits, existing search collectors, outer sorting and pagination; reviewed eligibility guards and changed test oracles.

• Frozen head CI reports successful Ubuntu unit tests, arm64 static analysis, UT coverage, and proxy/pessimistic multi-CN BVT.

• Local diff check passed: checks/1793142908251585041-3.json and diff-check.log.

• Source and discussion integrity audit passed: checks/1793143086619647942-5.json and evidence-audit-final.log. Initial audit rejected only the supplied patch’s omitted final newline.

• Saved discussion-assessment.json covering every discussion key, plus review-notes.md and review-evidence-manifest.json in the assigned diagnostic directory.

Limitations:

• No local Go tests or BVT rerun; raw CI logs and retained local-validation artifacts were not supplied.

• Author benchmark timings were not independently reproduced.

• Secondary keys do not guarantee selection among cutoff ties. Broader #30402 and original #31119 PMC performance requirements remain outside this PR’s closure.

[mergify]

🚫 Left the queue

2 h 21 min in the queue, with no time running CI.

So far

• ✅ Entered the queue Tue 27 Oct, 23:19 · rule main · triggered by rule Automatic queue on approval for main
• 🚫 Left the queue Wed 28 Oct, 01:40 · 235055d

Reason

The pull request #31487 has been manually updated

Tick the box to put this pull request back in the merge queue (same as @mergifyio queue).

• Requeue this pull request

_{Times in UTC · dashboard · how the merge queue works}