feat: tuple-encoded json word breaker and json_extract index probes - #27821
Merged
Conversation
…atrixorigin#27704) The json fulltext parser threw keys away: {"a":{"b":"XXX"}} indexed as the single token XXX, so nothing could tell {"b":"XXX"} from {"c":"XXX"}. This replaces it with a structure-aware breaker and teaches the optimizer to use it. ## Term encoding Each scalar leaf at path a.b....y.z is indexed as the order-preserving tuple ( z , V [, "a.b....y"] ) built with types.Packer. Element order is deliberate: tag first, then value, then the ancestor path, so prefix(tag) is every value under a key (range-scannable), prefix(tag,value) is that pair at ANY path, and the full tuple pins one path. Terms are RAW packed bytes, not hex: they carry 0x00 and BOOLEAN-mode metacharacters, so they never touch a pattern parser or a SQL literal. The parser is fulltext2-only for that reason - the v1 engine interpolates terms into SQL text. Two decisions the tests forced: - EVERY number encodes as float64. {"b":3} parses as TpCodeInt64 and {"b":3.0} as TpCodeFloat64, but JSON has one number type: encoding per width left {"b":3} unreachable from every numeric probe. - Date/Time/Datetime index as strings and Decimal under BOTH forms, because json_extract_string renders them and json_extract_float64 returns decimals. An extractable-but-unindexed leaf makes a probe drop rows. ## Options include_keys (default true) and include_full_path (default false), persisted in IndexAlgoParams and read identically by both build paths. TableConfig stores the "no keys" flag INVERTED so a config that never set it still indexes keys. ## Both build paths CREATE (fulltext2_create.rowTerms) and the incremental ISCP build (Fulltext2SqlWriter.rowText + CdcTokenizerWithJSONOptions) call ONE encoder. The CDC blob is already length-prefixed and CRC-checked, so the writer emits finished terms into it rather than flattened text - flattening discards the very keys this change adds. TestCreateAndIscpAgreeOnTerms asserts the two paths emit byte-identical (word, pos) pairs; a past divergence here made CDC-inserted json rows silently unsearchable. ## Optimizer json_extract_string|json_extract_float64(col,'$.path') <op> const, for =, >, >=, <, <=, gains an ANDed probe. The original predicate is retained and decides the answer, so the probe need only be a SUPERSET; every gate exists to keep that implication true: - json_extract_string = '3.14' probes the string AND float encodings, since it renders numeric leaves. - json_extract_string inequalities union the ordered string range with EVERY numeric term - the two orders disagree ("10" < "9"), so the numeric side cannot be narrowed without risking a dropped row. - Ranges are inclusive at both ends; a strict > returns one extra term the retained predicate removes. - Declined: wildcard paths, paths with no trailing key, non-constant operands, OR/NOT context, and value ranges on a full-path index (the path sorts after the value, so the range is not contiguous). The probe rides the ordinary MATCH surface (fulltext_match + a distinct mode), so the whole scan-to-TVF rewrite applies unchanged. ## Execution Only what could not be expressed already: Segment.TermRange / termDict.rangeTerms (no MATCH pattern can say "terms between X and Y") and the probe dispatch. sortedTerms is BUILD-side and nil on a loaded segment, so the range path needed the FST iterator too - without it every range silently returned zero rows against a persisted index. SearchBoolean and streamDisjunction are untouched. ## Tests BVT fulltext2_json_probe.sql (43/43) pairs every indexed query against an unindexed twin, so the assertion is equivalence rather than a row count. fulltext2_parser.sql's json section is rewritten for the new semantics: it now covers whole-value equality (a substring no longer matches), key significance, and numeric ranges, also twinned. Unit tests cover the leaf walk, the encoder, CREATE/ISCP parity, the payload codec, TermRange, and the optimizer rule. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QMQRVu998WixePcJCJvyA5
fengttt
requested review from
LeftHandCold,
XuPeng-SH,
aptend,
aunjgr and
cpegeric
as code owners
August 28, 2026 20:49
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? |
There was a problem hiding this comment.
Pull request overview
This PR upgrades the fulltext2 JSON parser from value-flattened tokenization to a structure-aware, tuple-encoded term format and adds an optimizer rewrite that turns eligible json_extract_string/json_extract_float64 comparisons into fulltext2 index probes (while retaining the original predicate for correctness). It also adds execution support for binary probe payloads and inclusive term-range scans needed by JSON range predicates, plus BVT/unit coverage to ensure CREATE and CDC/ISCP builds emit identical terms.
Changes:
- Add tuple term encoding for JSON leaves (optionally with ancestor path), shared by CREATE and ISCP build paths, with a binary-safe CDC term carrier.
- Add planner rule to synthesize
fulltext_match(..., JSONProbeMode, col)probes from supportedjson_extract_*comparisons and allow these probes on POSITION_FREE fulltext2 indexes. - Add fulltext2 execution support for JSON probe payload decoding and inclusive term-range expansion, plus new/updated BVT and unit tests.
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| test/distributed/cases/fulltext2/fulltext2_parser.sql | Updates parser BVT to reflect new JSON tuple semantics (comparison-driven, not free-text MATCH). |
| test/distributed/cases/fulltext2/fulltext2_parser.result | Updates expected output for the revised parser BVT. |
| test/distributed/cases/fulltext2/fulltext2_json_probe.sql | Adds BVT coverage asserting indexed-vs-unindexed equivalence for JSON extract probes/ranges. |
| test/distributed/cases/fulltext2/fulltext2_json_probe.result | Expected output for the new JSON probe BVT. |
| pkg/sql/plan/apply_indices.go | Wires JSON probe synthesis into the applyIndices flow before MATCH filter collection. |
| pkg/sql/plan/apply_indices_fulltext2.go | Allows JSONProbeMode queries on POSITION_FREE fulltext2 indexes. |
| pkg/sql/plan/apply_indices_fulltext_json.go | Implements json_extract comparison recognition, probe construction, and probe MATCH injection. |
| pkg/sql/plan/apply_indices_fulltext_json_test.go | Unit tests for probe recognition, bounds, declines, operand flipping, and term-shape handling. |
| pkg/sql/colexec/table_function/fulltext2_search.go | Plumbs JSONProbeMode into the fulltext2 search execution query struct. |
| pkg/sql/colexec/table_function/fulltext2_create.go | Uses the shared JSON tuple encoder on the CREATE build path before any flattening. |
| pkg/iscp/fulltext2_sqlwriter.go | Emits finished tuple terms into a binary carrier for ISCP/CDC builds and persists JSON term-shape config. |
| pkg/iscp/fulltext2_consumer.go | Switches CDC tokenization to a JSON-options-aware tokenizer to match CREATE behavior. |
| pkg/fulltext2/termdict.go | Adds FST-backed inclusive range term enumeration (rangeTerms). |
| pkg/fulltext2/storage.go | Extends TableConfig with persisted JSON term-shape flags (no-keys/full-path). |
| pkg/fulltext2/segment.go | Adds inclusive in-memory term range selection over build-side sorted terms. |
| pkg/fulltext2/search_cache.go | Adds JSONProbe dispatch for both top-k and streaming paths. |
| pkg/fulltext2/query.go | Adds CdcTokenizerWithJSONOptions to decode binary term carriers for tuple JSON indexes. |
| pkg/fulltext2/plugin/compile/compile.go | Reads JSON term-shape options from IndexAlgoParams into TableConfig for build SQL generation. |
| pkg/fulltext2/jsonterm.go | Implements tuple term encoding, probe term helpers, CDC carrier codec, and term-shape options. |
| pkg/fulltext2/jsonterm_test.go | Tests term encoding invariants, truncation symmetry, CREATE/ISCP parity, and payload codec behaviors. |
| pkg/fulltext2/jsonprobe.go | Implements JSON probe payload encoding/decoding and query building via disjunction of resolved terms. |
| pkg/fulltext2/boolean.go | Adds segment term-range expansion that works for both loaded (FST) and build-side representations. |
| pkg/container/bytejson/leafiter.go | Adds a structure-aware leaf iterator emitting typed leaves plus tag/ancestor path. |
| pkg/container/bytejson/leafiter_test.go | Unit tests for leaf iteration (keys, paths, arrays, determinism, early stop). |
| pkg/catalog/secondary_index_utils.go | Adds catalog constants for JSON term-shape algo params. |
| docs/design/json_tuple_wordbreaker.md | Design doc describing tuple encoding, probe rewrite contract, and build/query implications. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+191
to
+192
| nlo, nhi := fulltext2.JSONNumericTermBounds(tag) | ||
| p.Ranges = []jsonTermRange{r, {Lo: nlo, Hi: nhi}} |
Comment on lines
+41
to
+42
| -- a numeric leaf reached through json_extract_string: the constant is probed | ||
| -- under BOTH encodings, so row 7 (the NUMBER 3.14) must not be lost |
Comment on lines
+176
to
+180
| ### 4.3 Surface | ||
|
|
||
| ```sql | ||
| CREATE FULLTEXT INDEX idx ON t(j) WITH PARSER json; -- keys on, leaf-only | ||
| CREATE FULLTEXT INDEX idx ON t(j) WITH PARSER json, INCLUDE_FULL_PATH = TRUE; -- keys on, full path |
Comment on lines
+3
to
+6
| Status: **encoder + both build paths implemented; optimizer rule implemented but | ||
| not yet wired into `applyIndices`** (the execution side has no binary-probe or | ||
| term-range support, so emitting the conjunct would break queries rather than | ||
| accelerate them). See §7 for what is done and what remains. |
Review feedback, and it was right: json_extract_string and json_extract_float64
are DISJOINT on leaf type. Verified against the server —
json_extract_string('{"v":3.14}','$.v') IS NULL, and json_extract_float64 is
NULL for a string leaf. I had assumed json_extract_string renders numbers and
built a two-encoding probe around that.
Consequences:
- Equality no longer probes both encodings. `json_extract_string(...) = '3.14'`
can only be true for the STRING "3.14", so the float term was a term no
qualifying document can hold.
- The string RANGE no longer unions the numeric side. That was not merely
redundant: expanding [-Inf,+Inf] materializes the tag's entire numeric
vocabulary, so a selective string predicate became a near-full term scan.
- A decimal leaf is numeric to both extractors, so it is indexed only in the
numeric form instead of both.
The BVT already encoded the true behaviour (the unindexed twin returned only
the string row) — the comment beside it asserted the opposite, which is exactly
the trap the twin exists to catch. Comment corrected.
Also fixed, from the same review:
- docs: the Status note still said the rule was unwired and execution lacked
range support; both landed in the previous commit.
- docs: INCLUDE_KEYS / INCLUDE_FULL_PATH were documented as SQL options, but the
grammar has no such index option — only POSITION_FREE. Marked as proposed,
documented the surface that actually exists, and added the DDL + SHOW CREATE
round-trip to the remaining-work list. Everything below the grammar already
reads both params.
CI: pessimistic_transaction/fulltext2/fulltext2_bugfix.sql queried a json index
with free-text MATCH, which this feature removes by design. Its json cases are
rewritten as json_extract probes, keeping the original intent — proving the CDC
consumer indexed a T_json column and both columns of a multi-column json index.
That case waits on cdc_tail, so it now covers the asynchronous incremental build
that fulltext2_json_probe.sql deliberately does not assert.
cases/fulltext/* is unaffected: it builds v1 indexes, and the tuple breaker is
fulltext2-only.
Both fulltext2 suites pass; the seven affected packages build, vet and test
clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QMQRVu998WixePcJCJvyA5
…ixone into feat-json-tuple-wordbreaker
The SCA job's license-eye check failed on this one file; the header is required on every source file. Also folds its two single-line imports into one block. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QMQRVu998WixePcJCJvyA5
…dbreaker # Conflicts: # pkg/sql/plan/apply_indices_fulltext2.go
A json probe is a PREFILTER the optimizer injected: it returns a SUPERSET that the retained predicate then narrows. Truncating it to k candidates therefore yields fewer than k final rows and silently drops qualifying ones. Before the upstream merge this was structural — buildFullTextCandidateLimit refused any pushdown while scanNode.FilterList was non-empty, and a probe always leaves its own predicate there. matrixorigin#27252 added a residual-filter path that can now push a LIMIT past a residual filter. A probe still does not qualify, because fulltext2ConjunctiveCandidateLimitEligible requires FULLTEXT_BOOLEAN mode and a probe's mode is JSONProbeMode — but that is incidental, and widening conjunctive eligibility later would turn it into a wrong-results bug with no test to catch it. Refuse probes explicitly, and pin the refusal. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QMQRVu998WixePcJCJvyA5
fengttt
requested review from
XuPeng-SH and
aunjgr
and
a lite review from Copilot
August 30, 2026 19:54
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 41 out of 44 changed files in this pull request and generated 2 comments.
Suppressed comments (1)
docs/design/json_tuple_wordbreaker.md:418
- Decision #6 states that json_extract_string probes the OR of plausible encodings because it renders numeric leaves, but the current implementation treats json_extract_string and json_extract_float64 as disjoint on leaf type (json_extract_string returns NULL for numeric/decimal leaves). This decision summary should be updated to reflect the implemented behavior so it remains a reliable reference for future changes.
6. **`json_extract_string` probes the `OR` of plausible encodings.** Confirmed.
`= 'XXX'` (not numeric) stays a single exact string-term lookup; `= '3.14'`
probes the string term OR the float term, because
`json_extract_string` renders a numeric leaf and the predicate is genuinely
true for it. The disjunction is still a necessary condition, so the retained
predicate removes the extras. Without this the row would be dropped — a
Comment on lines
+97
to
+118
| // Date/Time/Datetime store their text and json_extract_string renders exactly | ||
| // those bytes (CompareByteJson groups them with TpCodeString for the same | ||
| // reason), so they index as strings. Leaving them out would make | ||
| // json_extract_string(...) = '2024-01-02' miss a document that satisfies it. | ||
| case TpCodeString, TpCodeDate, TpCodeTime, TpCodeDatetime: | ||
| return w.yield(Leaf{Tag: tag, Kind: LeafString, Str: bj.GetString()}) | ||
| case TpCodeInt64: | ||
| return w.yield(Leaf{Tag: tag, Kind: LeafInt64, I64: bj.GetInt64()}) | ||
| case TpCodeUint64: | ||
| return w.yield(Leaf{Tag: tag, Kind: LeafUint64, U64: bj.GetUint64()}) | ||
| case TpCodeFloat64: | ||
| return w.yield(Leaf{Tag: tag, Kind: LeafFloat64, F64: bj.GetFloat64()}) | ||
| case TpCodeDecimal: | ||
| // Numeric to both extractors: json_extract_float64 returns it as a | ||
| // number, json_extract_string returns NULL for it. Str carries the | ||
| // decimal text; the encoder parses it into the numeric form. | ||
| return w.yield(Leaf{Tag: tag, Kind: LeafDecimal, Str: bj.GetString()}) | ||
| default: | ||
| // TpCodeLiteral (true/false/null), Blob, Opaque and Bit are not indexed. | ||
| // json_extract_string can still render some of those, so a probe must not | ||
| // be synthesized for a column that may hold them — see the caller's gate. | ||
| return true |
Comment on lines
+273
to
+277
| **Type family — the one real trap left.** A JSON number `3.14` is indexed as a | ||
| float element, but `json_extract_string(j,'$.a.b') = '3.14'` is **true** for | ||
| that document (`json_extract_string` renders a numeric leaf, it does not return | ||
| NULL). Probing only the string encoding would drop that row — a wrong answer, | ||
| not a missed optimization. |
The repo's err-check gate forbids errors.New() in pkg/, and the SCA job failed on the sentinel the probe walk used to unwind when its sink stopped. Swapping in moerr would have satisfied the gate and kept the actual mistake: the sentinel was never an error, only a control-flow signal for an ordinary end (a cancelled query, or a full downstream). The term-walk callbacks now report whether to CONTINUE — forEachTermInRange on both the FST and build-side paths, and probeWalk — so an early stop needs no error at all. Verified with the gate's own command (`make err-check`) and the full `make static-check`: err-check clean, license-eye invalid: 0, golangci-lint 0 issues, exit 0. I had been running golangci-lint alone, which does not include either of the checks that failed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QMQRVu998WixePcJCJvyA5
TestBuildCreateOrReplaceViewRejectsRecursiveDefinition/future_AS_OF_timestamp
passed in CI and failed for anyone west of UTC.
AS OF TIMESTAMP is parsed in the MACHINE's zone — doResolveTimeStamp uses
time.LoadLocation("Local") — and converted with UnixNano, so a fixed literal is
not timezone-independent. '2262-04-11 23:47:16' sits on the int64-nanosecond
ceiling (2262-04-11 23:47:16.854775807 UTC): at or ahead of UTC it converts
under the ceiling and the case tests what it means to, but behind UTC it
converts PAST it, overflows to a negative value, and validateTimestampHint
rejects it as "invalid timestamp value" before the recursion check ever runs.
The assertion then compared the wrong error. CI runs UTC, so it never saw this.
Derive the literal from the ceiling in the local zone instead, so the expected
message is the same everywhere. A day of margin absorbs the widest real UTC
offset and any DST rule extrapolated into 2262, and formatting to second
precision only rounds down; the case still lands in April 2262, so it is still
the far-future timestamp it was written to be.
Verified across UTC-12..UTC+14 including half-hour offsets and a 30-minute DST
zone: UTC, America/Los_Angeles, America/New_York, Asia/Shanghai, Asia/Kolkata,
Pacific/Kiritimati, Etc/GMT+12, Australia/Lord_Howe, Europe/London all pass. As
a control, the old literal is invalid in exactly the zones behind UTC
(America/Los_Angeles, Etc/GMT+12) and valid in the rest.
This is an upstream test, unrelated to the json word breaker — split it out if
you would rather it went in on its own.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QMQRVu998WixePcJCJvyA5
Collaborator
|
Also resolves #27861. The merged change makes the recursive-view planner test's future |
7 tasks
7 tasks
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.
What type of PR is this?
Which issue(s) this PR fixes:
issue #27704
What this PR does / why we need it:
The json fulltext parser threw keys away.
{"a":{"b":"XXX"}}indexed as thesingle token
XXX, so nothing could distinguish{"b":"XXX"}from{"c":"XXX"}. This replaces the breaker with a structure-aware one and teachesthe optimizer to use it.
Design doc:
docs/design/json_tuple_wordbreaker.md.Term encoding
Each scalar leaf at
a.b.….y.zis indexed as the order-preserving tuple( z , V [, "a.b.….y"] )viatypes.Packer. The element order is the usefulpart:
prefix(tag)is every value under a key (range-scannable),prefix(tag,value)is that pair at any path, and the full tuple pins onepath — so leaf-only is a strict prefix of full-path.
Terms are raw packed bytes, not hex: they carry
0x00and BOOLEAN-modemetacharacters, so they never reach a pattern parser or a SQL literal. The
parser is therefore fulltext2-only (the v1 engine interpolates terms into SQL
text).
Two encoding rules the tests forced:
{"b":3}parses asTpCodeInt64and{"b":3.0}asTpCodeFloat64, but JSON has one number type — encoding perwidth left
{"b":3}unreachable from every numeric probe.json_extract_stringrenders them andjson_extract_float64returnsdecimals. An extractable-but-unindexed leaf makes a probe drop rows.
Options
include_keys(default true) andinclude_full_path(default false), persistedin
IndexAlgoParamsand read identically by both build paths.TableConfigstores the "no keys" flag inverted so a config that never set it still
indexes keys.
Both build paths use one encoder
CREATE (
fulltext2_create.rowTerms) and the incremental ISCP build(
Fulltext2SqlWriter.rowText+CdcTokenizerWithJSONOptions) call the sameencoder. The CDC blob is already length-prefixed and CRC-checked, so the writer
emits finished terms into it rather than flattened text — flattening discards
the very keys this change adds.
TestCreateAndIscpAgreeOnTermsasserts the twopaths emit byte-identical
(word, pos)pairs; a past divergence here madeCDC-inserted json rows silently unsearchable.
Optimizer
json_extract_string|json_extract_float64(col,'$.path') <op> constfor=,>,>=,<,<=gains an ANDed index probe. The original predicate isretained and decides the answer, so the probe need only be a superset. Every
gate exists to keep that implication true:
json_extract_string = '3.14'probes the string and float encodings,since it renders numeric leaves.
json_extract_stringinequalities union the ordered string range with everynumeric term: the two orders disagree (
"10" < "9"), so the numeric sidecannot be narrowed without risking a dropped row.
>returns one extra term theretained predicate removes.
OR/NOT context, and value ranges on a full-path index (the path sorts after
the value, so the range is not contiguous).
The probe rides the ordinary MATCH surface (
fulltext_match+ a distinct mode),so the whole scan-to-TVF rewrite applies unchanged.
Execution
Only what could not already be expressed:
Segment.TermRange/termDict.rangeTerms(no MATCH pattern can say "terms between X and Y") and theprobe dispatch — about 80 lines.
SearchBooleanandstreamDisjunctionareuntouched.
Worth flagging:
sortedTermsis the build-side term list and is nil on aloaded segment, so the range path needed the FST iterator too. Without it every
range silently returned zero rows against a persisted index — caught only
because the BVT compares against an unindexed twin.
Behaviour change
WITH PARSER jsonno longer indexes flattened values as ngrams, so free-textMATCH(j) AGAINST('XXX')over a json index no longer matches. That is theintended replacement (
include_keys = falsekeeps value-only indexing).fulltext2_parser.sql's json section is rewritten accordingly.json_valueisuntouched.
Testing
fulltext2_json_probe.sql(43/43) — every indexed query is pairedwith an unindexed twin, so the assertion is equivalence, not a row count:
equality, nested paths, arrays, the numeric/string dual probe, all four range
operators, reversed operands, and AND/OR/NOT contexts.
fulltext2_parser.sql— json section rewritten for the new semantics(whole-value equality, substring no longer matching, key significance, numeric
ranges), also twinned. Full
fulltext2/suite passes.TermRange,and the optimizer rule (including 11 decline cases).
pkg/catalog,pkg/container/bytejson,pkg/fulltext2,pkg/fulltext2/plugin/compile,pkg/iscp,pkg/sql/colexec/table_function,pkg/sql/planall build, vet andgofmt clean.
A full local BVT run is in progress; I will report the result on this PR. Rows
inserted after CREATE INDEX are indexed asynchronously, so their visibility is
not asserted in BVT (a fixed sleep would be flaky) — that parity is covered
deterministically by the unit test instead.