Skip to content

UNLOCK SNAPSHOT formatting missing spaces in both ALTER and SYSTEM variants #101723

@clickgapai

Description

@clickgapai

Found via ClickGap automated review. Please close or comment if this is incorrect or needs adjustment.

Describe what's wrong

formatQuery('ALTER TABLE t UNLOCK SNAPSHOT ''snap1'' FROM S3(...)') produces 'snap1'FROM (no space before FROM). formatQuery('SYSTEM UNLOCK SNAPSHOT ''snap1'' FROM S3(...)') produces SNAPSHOT'snap1' (no space before quoted name).

Root cause: ASTAlterQuery.cpp:266: ostr << "FROM "; should be ostr << " FROM ";. ASTSystemQuery.cpp:383: ostr << quoteString(backup_name); should be ostr << " " << quoteString(backup_name);.

Why we believe this is a bug: ASTAlterQuery.cpp:266 outputs 'FROM ' without a leading space after quoteString(). ASTSystemQuery.cpp:383 outputs quoteString(backup_name) immediately after the type keyword 'UNLOCK SNAPSHOT' without a space separator.

Affected locations:

  • src/Parsers/ASTAlterQuery.cpp:266 — Missing space before FROM in UNLOCK_SNAPSHOT ALTER formatting
  • src/Parsers/ASTSystemQuery.cpp:383 — Missing space before backup_name in UNLOCK_SNAPSHOT SYSTEM formatting

Impact: formatQuery produces improperly formatted SQL for UNLOCK SNAPSHOT commands. While still parseable, the output is malformed (missing whitespace between tokens).

Does it reproduce on most recent release?

Yes — confirmed on current master (commit fc7ad06dc31f).

How to reproduce

-- Test: Verify UNLOCK SNAPSHOT formatting produces properly spaced SQL

-- ALTER TABLE UNLOCK SNAPSHOT formatting: should have space before FROM
SELECT formatQuery('ALTER TABLE t UNLOCK SNAPSHOT ''snap1'' FROM S3(''http://example.com'')');

-- SYSTEM UNLOCK SNAPSHOT formatting: should have space after SNAPSHOT keyword before the name
SELECT formatQuery('SYSTEM UNLOCK SNAPSHOT ''snap1'' FROM S3(''http://example.com'')');

Expected behavior

ALTER TABLE t\n    (UNLOCK SNAPSHOT 'snap1' FROM S3('http://example.com'))
SYSTEM UNLOCK SNAPSHOT 'snap1' FROM S3('http://example.com')

Error message and/or stacktrace

ALTER TABLE t\n    (UNLOCK SNAPSHOT \'snap1\'FROM S3(\'http://example.com\'))
SYSTEM UNLOCK SNAPSHOT\'snap1\' FROM S3(\'http://example.com\')

Additional context

Open risks:

  • ASTAlterCommand::snapshot_desc is not initialized to nullptr (line 226 of ASTAlterQuery.h), not cloned in clone() method, and not listed in forEachPointerToChild(). This could cause crashes when UNLOCK_SNAPSHOT ASTs are cloned (e.g., for ON CLUSTER DDL).

Suggested fix: ASTAlterQuery.cpp:266: change ostr << "FROM "; to ostr << " FROM ";. ASTSystemQuery.cpp:383: change ostr << quoteString(backup_name); to ostr << " " << quoteString(backup_name);.

Analysis details: Confidence HIGH | Severity P3 | Testability: STATELESS_SQL

Found during automated review of PR #77308.


ClickGapAI · Confidence: HIGH · Severity: P3 · Finding: h_pr77308_001

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions