feat(parser): ClickHouse SETTINGS/TTL/INSERT FORMAT tail clauses (#482)

[ajitpratap0]

Summary

Three ClickHouse tail-clause gaps from the QA sweep (#482).

• `SETTINGS k = v [, k = v]...` at the end of `CREATE TABLE` (after ENGINE + ORDER BY) and at the end of `SELECT`. Parse-only; settings are consumed but not modeled on the AST.
• `TTL expr` clause in `CREATE TABLE` trailing-clause position, interoperating with the existing ORDER BY / PARTITION BY / PRIMARY KEY / SAMPLE BY loop.
• `INSERT INTO t [(cols)] FORMAT ` — short-circuited before the VALUES/SELECT requirement so formats like `JSONEachRow`, `CSV`, `TabSeparated` work without a SQL payload. Also supports `INSERT ... VALUES (...) FORMAT `.

Test plan

• `TestClickHouseSettingsTTLFormat`: 6 shapes — CREATE TABLE + SETTINGS, + TTL, TTL + SETTINGS chained, SELECT + SETTINGS, INSERT VALUES + FORMAT, INSERT + FORMAT without VALUES
• `go test -race ./pkg/sql/parser/ ./pkg/gosqlx/` green

Out of scope

• AST modeling for SETTINGS/TTL (currently discarded)
• Format payload ingestion for `INSERT ... FORMAT`

Part of #482.

🤖 Generated with Claude Code

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
gosqlx-website	Ready	Preview, Comment	Apr 8, 2026 5:31am

[github-actions]

Claude finished @ajitpratap0's task —— View job

---

✅ Code Review Complete

I've conducted a thorough review of the ClickHouse SETTINGS/TTL/FORMAT implementation. Here's my comprehensive feedback:

🎯 Overall Assessment: LGTM with Suggestions

The implementation is solid and well-designed. It follows existing patterns, maintains code quality standards, and correctly implements the parse-only approach for these ClickHouse-specific features.

---

📋 Detailed Review

✅ Strengths

1. Excellent Dialect-Aware Implementation

if p.dialect == string(keywords.DialectClickHouse) && p.isTokenMatch("SETTINGS")

• Proper dialect checking prevents parsing conflicts with other SQL dialects
• Follows established patterns from existing ClickHouse features

2. Robust Error Handling

if err := p.skipClickHouseClauseExpr(); err != nil {
    return nil, err
}

• Consistent error propagation throughout parsing logic
• Uses existing skipClickHouseClauseExpr() helper for TTL expressions

3. Smart INSERT FORMAT Implementation

// Short-circuit here — there is no VALUES or SELECT to follow.
if p.dialect == string(keywords.DialectClickHouse) && 
   strings.EqualFold(p.currentToken.Token.Value, "FORMAT") {

• Correctly handles both INSERT ... FORMAT and INSERT ... VALUES ... FORMAT patterns
• Early return prevents unnecessary VALUES/SELECT parsing for format-based inserts

4. Comprehensive Test Coverage

• Tests cover all 6 major patterns: CREATE TABLE with SETTINGS, TTL, chained clauses, SELECT with SETTINGS, and both INSERT FORMAT variants
• Uses proper gosqlx.ParseWithDialect() for dialect-specific testing

---

🔧 Minor Suggestions for Improvement

1. Enhanced Error Context (ddl.go:311-322)

// Current implementation is basic
for {
    t := p.currentToken.Token.Type
    val := strings.ToUpper(p.currentToken.Token.Value)
    if t == models.TokenTypeEOF || t == models.TokenTypeSemicolon {
        break
    }
    // ... basic keyword checks ...
    p.advance()
}

Suggestion: Add validation for SETTINGS syntax:

// Could validate k=v pairs structure:
// SETTINGS key1 = value1, key2 = value2
if !p.isIdentifier() {
    return goerrors.InvalidSyntaxError("expected setting name", p.currentLocation(), "")
}

2. Test Robustness (clickhouse_settings_ttl_format_test.go)

// Current: Basic parse success testing
if _, err := gosqlx.ParseWithDialect(q, keywords.DialectClickHouse); err != nil {
    t.Fatalf("parse failed: %v", err)
}

Suggestion: Add negative test cases:

negativeQueries := map[string]string{
    "invalid_settings": `CREATE TABLE t (id Int64) SETTINGS = invalid`,
    "missing_ttl_expr": `CREATE TABLE t (id Int64) TTL`,
    "invalid_format":   `INSERT INTO t FORMAT`,
}

3. Code Documentation
Consider adding brief comments explaining the parse-only nature:

// ClickHouse SETTINGS parsing: consume but don't model on AST (parse-only)
if p.isTokenMatch("SETTINGS") {

---

🚀 Performance Considerations

Excellent Performance Characteristics:

• ✅ Uses strings.EqualFold() for case-insensitive matching (standard pattern)
• ✅ Leverages existing skipClickHouseClauseExpr() helper (no duplication)
• ✅ Early returns in INSERT FORMAT prevent unnecessary parsing
• ✅ Minimal memory allocation (parse-only, no AST nodes created)

---

🔒 Security Assessment

No Security Concerns Identified:

• ✅ No user input evaluation or execution
• ✅ Parse-only implementation with proper bounds checking
• ✅ Uses existing error handling patterns
• ✅ Dialect-scoped to prevent injection across SQL flavors

---

🧪 Test Quality

Test Coverage: 95/100

• ✅ Covers all main syntax patterns
• ✅ Uses proper dialect-aware parsing
• ✅ Tests integration points (TTL + SETTINGS chaining)
• ✅ Includes edge cases (VALUES + FORMAT, bare FORMAT)

Missing (Minor):

• ❓ Negative test cases for malformed syntax
• ❓ Tests verifying SETTINGS don't appear in non-ClickHouse dialects

---

📦 Integration Quality

Excellent Integration:

• ✅ Follows established skipClickHouseClauseExpr() pattern from existing code
• ✅ Maintains parser loop structure in parseCreateTable()
• ✅ Consistent with other ClickHouse dialect features
• ✅ No breaking changes to public APIs

---

🎯 Recommendation

✅ APPROVE - This is a well-crafted implementation that:

• Correctly implements the specified features
• Follows established code patterns and conventions
• Includes comprehensive test coverage
• Maintains performance and security standards

The minor suggestions above are optional improvements that don't affect the core functionality. The code is ready for merge as-is.

Priority: The suggested test enhancements could be addressed in a follow-up PR if desired, but are not blockers for this implementation.

---

Job Run