Skip to content

FIX: CREATE TABLE AS SELECT with MATCH...AGAINST drops string-literal quotes during rewrite#25683

Open
cpegeric wants to merge 4 commits into
matrixorigin:mainfrom
cpegeric:issue_24823
Open

FIX: CREATE TABLE AS SELECT with MATCH...AGAINST drops string-literal quotes during rewrite#25683
cpegeric wants to merge 4 commits into
matrixorigin:mainfrom
cpegeric:issue_24823

Conversation

@cpegeric

Copy link
Copy Markdown
Contributor

What type of PR is this?

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

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

cpegeric and others added 2 commits July 13, 2026 16:05
…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-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 →

@matrix-meow matrix-meow added the size/S Denotes a PR that changes [10,99] lines label Jul 13, 2026
@cpegeric cpegeric changed the title fFIX: CREATE TABLE AS SELECT with MATCH...AGAINST drops string-literal quotes during rewrite FIX: CREATE TABLE AS SELECT with MATCH...AGAINST drops string-literal quotes during rewrite Jul 13, 2026

@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.

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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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