Skip to content

fix: tokenize non-null fulltext columns independently - #26609

Merged
XuPeng-SH merged 3 commits into
matrixorigin:mainfrom
VioletQwQ-0:codex/issue-26557-fulltext-null-columns
Aug 3, 2026
Merged

fix: tokenize non-null fulltext columns independently#26609
XuPeng-SH merged 3 commits into
matrixorigin:mainfrom
VioletQwQ-0:codex/issue-26557-fulltext-null-columns

Conversation

@VioletQwQ-0

Copy link
Copy Markdown
Collaborator

What type of PR is this?

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

Which issue(s) this PR fixes:

issue #26557

What this PR does / why we need it:

fulltext_index_tokenize currently returns before tokenization when any indexed content column is NULL. In a multi-column FULLTEXT index this silently omits the entire document, even when another indexed column contains searchable content.

This PR makes the smallest behavior change at the shared tokenizer boundary:

  • skip each NULL content column independently;
  • preserve the order, delimiter, token position, and JSON byte offset of participating non-NULL columns;
  • continue to emit no token/doclen row when all content columns are NULL;
  • leave errors from malformed non-NULL JSON, datalink, and parser input unchanged.

The shared boundary covers index creation and synchronous/post-DML/asynchronous maintenance without planner, catalog, or index-plugin restructuring.

Coverage

  • Unit matrix: ordinary/default, ngram, gojieba, json, and json_value; left/right/all/neither NULL; ordinary and const-NULL vectors.
  • DML/BVT: CREATE INDEX, omitted-column INSERT, (NULL, value) <-> (value, NULL) UPDATE, all-NULL removal, revival, DELETE, and async INSERT.
  • Existing TF-IDF/BM25/gojieba results are updated because previously omitted mixed-NULL documents now correctly participate in the corpus.

Exact-head local evidence for 5d4b02ac64a9895df2a284fe978e77098225eaf5:

  • TestFullTextTokenizeSkipsNullContentColumns -race -count=69: PASS
  • TestFullTextTokenizeSkipsConstNullContentColumn -race -count=100: PASS
  • ./pkg/sql/colexec/table_function -race -count=1: PASS
  • make build: PASS
  • mo-tester fulltext, fulltext_bm25, fulltext_update_consistency, gojieba: 715/715 statements PASS
  • git diff --check: PASS
  • preflight: PASS review=PASS, exact diff hash cb8b05c355a583a8b9b15fd407a6354f20964a3db7ac4f9987e61d233cbccfe4

Existing-index governance

This code fix is forward-maintenance only; it intentionally does not scan or mutate existing hidden FULLTEXT index tables. A multi-column index populated before the fix may already lack documents whose indexed row had a partial NULL and must be rebuilt after every CN and async-index consumer is on the fixed version.

Inventory candidate multi-column FULLTEXT indexes before rollout:

SELECT d.datname, t.relname, i.name,
       MIN(i.algo_params) AS algo_params,
       COUNT(*) AS indexed_columns
FROM mo_catalog.mo_indexes i
JOIN mo_catalog.mo_tables t ON i.table_id = t.rel_id
JOIN mo_catalog.mo_database d ON i.database_id = d.dat_id
WHERE i.algo = 'fulltext'
GROUP BY d.datname, t.relname, i.table_id, i.name
HAVING COUNT(*) > 1
ORDER BY d.datname, t.relname, i.name;

For each affected index, capture SHOW CREATE TABLE so the index name, ordered columns, parser, and async option are preserved. In a maintenance window, use supported DDL to drop and recreate the FULLTEXT index; do not edit mo_catalog or hidden index tables directly. There is no automatic migration in this PR.

Rollback caveat: rolling back the binary does not repair or revert index contents. Old writers can again omit partial-NULL documents, so pause affected FULLTEXT DML during rollback and rebuild after returning to the fixed version.

QA handoff

Validate on a Proxy + at least 2 CN deployment after all writers use the fixed build:

  1. Exercise the six parser modes with left NULL, right NULL, neither NULL, and all NULL.
  2. Exercise index creation plus INSERT/UPDATE/DELETE and async maintenance.
  3. Create a mixed-NULL index on the old version, upgrade, confirm the existing index remains incomplete before rebuild, rebuild it, then confirm the surviving column becomes searchable.
  4. Confirm all-NULL rows remain absent and corpus counts/scores include partial-NULL documents only after rebuild.

BVT: YES

QA required: YES. Keep issue #26557 open until the tested version/environment, rebuild evidence, and terminal PASS are recorded.

@qodo-code-review

Copy link
Copy Markdown

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 XuPeng-SH left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deep review complete. The shared tokenizer now skips NULL content columns independently while preserving participating-column order, delimiter and position behavior, JSON offsets, all-NULL zero-output semantics, and malformed non-NULL error propagation. Create-index, synchronous/post-DML, and async ISCP paths all converge on this boundary; existing-index rebuild and mixed-version rollout caveats are documented. Focused regressions passed under race at count 50, the full table_function package passed under race, fulltext/indexplugin/ISCP packages passed, build and vet passed, diff check passed, and current CI is green.

@XuPeng-SH
XuPeng-SH merged commit 7524202 into matrixorigin:main Aug 3, 2026
20 of 23 checks passed
@VioletQwQ-0
VioletQwQ-0 deleted the codex/issue-26557-fulltext-null-columns branch August 3, 2026 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/bug Something isn't working size/M Denotes a PR that changes [100,499] lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants