FIX: CREATE TABLE AS SELECT with MATCH...AGAINST drops string-literal quotes during rewrite#25683
FIX: CREATE TABLE AS SELECT with MATCH...AGAINST drops string-literal quotes during rewrite#25683cpegeric wants to merge 4 commits into
Conversation
…4823) CREATE TABLE AS SELECT re-serializes the SELECT (build_ddl.go RawSQL, deparsed with WithSingleQuoteString). FullTextMatchExpr.Format wrote the AGAINST pattern with raw WriteString, dropping the surrounding quotes, so a query like CREATE TABLE ctas_t AS SELECT id, MATCH(body) AGAINST('防水' IN BOOLEAN MODE) sc FROM ft WHERE MATCH(body) AGAINST('防水' IN BOOLEAN MODE); re-parsed as AGAINST(防水 ...) -> syntax error 1105. The pattern is stored unquoted (search_pattern: STRING strips the quotes), so Format must re-quote it. Emit it via WriteValue(P_char, FormatString(pattern)) exactly like NumVal string literals: quoted+escaped in the CTAS/restore context, unchanged (raw) in the default human-readable context — so existing MATCH roundtrip tests are unaffected. Regression: the matrixorigin#24823 query + an embedded-single-quote case added to the WithSingleQuoteString roundtrip suite (TestSQLStringFmt). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…24823) End-to-end regression for the deparse fix: builds a gojieba fulltext index and runs CREATE TABLE AS SELECT with MATCH(body) AGAINST('防水' IN BOOLEAN MODE) in both the projection (scored) and WHERE, plus WHERE-only variants. Before the fix this errored with syntax 1105 (dropped pattern quotes on CTAS re-serialization); now it creates the table with the correct matching rows. mo-tester run mode: 14/14. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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? |
XuPeng-SH
left a comment
There was a problem hiding this comment.
This still needs changes.
The CTAS path is only fixed because its formatter context enables quoteString, but the underlying FullTextMatchExpr formatter is still invalid under the default tree.String path.
Current code writes the AGAINST pattern through WriteValue(P_char, FormatString(node.Pattern)). That only adds quotes when the FmtCtx has quoteString or singleQuoteString enabled. On the default formatter path, it still emits the pattern bare.
I verified this directly on the latest head with a tiny parser round-trip program:
- tree.String(...) still prints: MATCH (body) AGAINST (防水 IN BOOLEAN MODE)
- only tree.StringWithOpts(..., WithSingleQuoteString()) prints: MATCH (body) AGAINST ('防水' IN BOOLEAN MODE)
So the PR does not actually fix the node's default deparse behavior; it only fixes contexts that happen to opt into string quoting. Because tree.String(...) is used in many other places across the codebase, any other current or future caller that serializes MATCH ... AGAINST through the default path can still generate broken SQL.
Suggestion: make FullTextMatchExpr.Format emit a properly quoted SQL string literal unconditionally (or otherwise ensure the default formatter path round-trips), and add a regression in the default tree.String / TestValid path, not only in the WithSingleQuoteString test path.
…parse path (matrixorigin#24823) FullTextMatchExpr.Format emitted the AGAINST pattern via ctx.WriteValue(P_char, ...), which only quotes when the FmtCtx opts in (quoteString/singleQuoteString). The default tree.String() path does not opt in, so the pattern was written bare (AGAINST (防水 ...)) — invalid SQL. Any re-serialization through the default path (CREATE TABLE AS SELECT, view expansion, etc.) produced un-reparseable SQL; the CTAS case only worked because it happened to enable string quoting. The pattern is ALWAYS a string literal (unlike the polymorphic NumVal), so emit it single-quoted and escaped unconditionally. Now the node round-trips on every formatter path. Regression coverage on the DEFAULT path: correct the six validSQL expectations in TestValid (which run through tree.String) from bare to quoted, and add TestFullTextMatchDeparseRoundTrip (default-path deparse -> re-parse -> idempotent), covering CJK and an embedded single quote. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
What type of PR is this?
Which issue(s) this PR fixes:
issue #24823
What this PR does / why we need it:
fixed. CREATE TABLE AS SELECT with MATCH...AGAINST drops string-literal quotes during rewrite