columnar: pin CSE FTS Spec v1 on cloud-engine - #11026
Conversation
Signed-off-by: 3pointer <l3pointer@foxmail.com>
Signed-off-by: 3pointer <l3pointer@foxmail.com>
|
Skipping CI for Draft Pull Request. |
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review. 📝 WalkthroughWalkthroughThe change updates the cloud-storage-engine submodule commit and changes the patched tantivy repository and branch in the tiflash-columnar-hub Cargo configuration. ChangesDependency and submodule updates
Estimated code review effort: 1 (Trivial) | ~5 minutes Merge Risk: 🔵 Low · up to The PR pins a new CSE revision and refreshes shared Rust dependency metadata; it is mergeable with owner awareness, but the private submodule commit and OpenSSL lockfile alignment should be confirmed because a mismatch could fail the columnar compatibility check. Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Signed-off-by: 3pointer <l3pointer@foxmail.com>
Signed-off-by: 3pointer <l3pointer@foxmail.com>
Signed-off-by: 3pointer <l3pointer@foxmail.com>
|
@glorv: adding LGTM is restricted to approvers and reviewers in OWNERS files. DetailsIn response to this: Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@yongman @JaySon-Huang PTAL |
|
@glorv @3pointer pls fix the compile error under nextgen columnar compile option |
|
@3pointer and create an issue for the behavior changes pls |
|
Please add compatible test result or report for legacy fts data. |
Legacy FTS data compatibility reportFTS files created by the old single-column stack remain readable and writable after rolling TiFlash → CSE → TiDB to Spec v1, after turning off legacy field layout, after creating new multi-column indexes in the same cluster, and after a full process restart. The application query ScopeThis report is about on-disk legacy FTS data, not about new multi-column DDL.
Upgrade orderSame order as the PR description: land CSE + TiFlash first, only then let TiDB emit multi-field FTS DDL.
Legacy tableCreated only at S0; never dropped; CREATE TABLE fts_legacy_single (
id BIGINT PRIMARY KEY,
bucket INT NOT NULL,
content TEXT NOT NULL,
FULLTEXT INDEX idx_legacy_content(content) WITH PARSER STANDARD
);
EXPLAIN after restart still uses the single column (not rewritten to a multi-field expression): A smaller single-column table ( Observed query resultsProbe SQL used at every stage: SELECT COUNT(*) FROM fts_legacy_single; -- rows
SELECT COUNT(*) FROM fts_legacy_single WHERE fts_match_word('legacytok42', content); -- MATCHExpected MATCH while the S0 baseline is intact: 1. Row count and MATCH hits per stage
TopK at LEGACY_READ_BEFORE_MULTI (actual 10 ids, all Restart EXPLAIN still scans the post-write table ( Restart Region ID = 3001 (was 28 at S0). Both FTS index names still present: 2. Rolling I/U/D on the legacy index (one throwaway row per stage)Each cycle: INSERT one row → UPDATE its token → DELETE it → restore baseline. MATCH is
Example S1 statements (from INSERT INTO fts_legacy_single VALUES (800000001,999,'legacyrws1 unique document 800000001');
SELECT id FROM fts_legacy_single WHERE fts_match_word('legacyrws1',content); -- actual 800000001
UPDATE fts_legacy_single SET content='legacyrws1after unique document 800000001' WHERE id=800000001;
SELECT id FROM fts_legacy_single WHERE fts_match_word('legacyrws1after',content); -- actual 800000001
SELECT id FROM fts_legacy_single WHERE fts_match_word('legacyrws1',content); -- actual empty
DELETE FROM fts_legacy_single WHERE id=800000001;
SELECT id FROM fts_legacy_single WHERE fts_match_word('legacyrws1after',content); -- actual empty3. Bulk write after multi-column indexes exist (not restored)
4. Restart (stop cluster, bring S3 binaries back)
ConclusionLegacy single-column FTS data written by CSE
No rebuild of old FTS files is required for this upgrade path. |
|
[FORMAT CHECKER NOTIFICATION] Notice: To remove the 📖 For more info, you can check the "Contribute Code" section in the development guide. |
|
|
@3pointer: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: glorv, JinheLin, yongman The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What problem does this PR solve?
Issue Number: N/A
Problem Summary:
TiDB/CSE FTS Spec v1 uses
full_text_index.index_fieldsas the canonical ordered physical field layout. CSE now owns parsing, validation, and conversion of that metadata. This PR now pins the latestfts_spec_v1after itscloud-enginerebase (36128849380c), including match-expression query protocol support from tidbcloud/cloud-storage-engine#5801.The production TiFlash path builds CSE through
contrib/tiflash-columnar-hub, so TiFlash only needs to pin that CSE revision. TiFlash C++ does not need to parse or validate the FTS fields.What is changed and how it works?
contrib/cloud-storage-engineto36128849380c, the latestfts_spec_v1head after thecloud-enginerebase.pingcap/tantivy:cse-0.22.1and refreshCargo.lock. This parent-workspace patch is required because Cargo ignores a child workspace's[patch.crates-io]when CSE is built by columnar hub.column_idhandling, Analyzer parameters, andenable_bm25remain in CSE.Metadata compatibility:
index_fields; oldfilter_columnsand transitionalfieldsare not used as the physical layout.parser_typeremains readable in CSE for rolling upgrades of existing single-field indexes.Deployment order:
Check List
Tests
Completed locally on Apple arm64 (revalidated with CSE
36128849380c):cargo check --locked --manifest-path hub-runtime/Cargo.tomlmake debug, producing an arm64libtiflash_proxy.dylibcargo fmt --all -- --checkPending:
Side effects
Documentation
Release note
Summary by CodeRabbit