Skip to content

refactor: route ClickHouse partition SQL through the driver's dialect-aware quoting#1503

Merged
datlechin merged 1 commit into
mainfrom
fix/clickhouse-partition-sql-escaping
May 30, 2026
Merged

refactor: route ClickHouse partition SQL through the driver's dialect-aware quoting#1503
datlechin merged 1 commit into
mainfrom
fix/clickhouse-partition-sql-escaping

Conversation

@datlechin

Copy link
Copy Markdown
Member

Summary

Phase 4 (R-007): ClickHousePartsView built four SQL statements with hand-rolled escaping — backtick-doubling for the table identifier and single-quote-doubling for user-supplied partition values. This routes all four through the driver's dialect-aware quoteIdentifier(_:) / escapeStringLiteral(_:) instead:

  • OPTIMIZE TABLE
  • ALTER TABLE ... DROP PARTITION
  • ALTER TABLE ... DETACH PARTITION
  • the system.parts lookup (table name used as a string literal)

Risk Addressed

  • R-007: scattered, hand-rolled SQL escaping (injection surface). Partition values are user input.

Correctness

PluginDriverAdapter forwards quoteIdentifier/escapeStringLiteral to the ClickHouse plugin, which overrides quoteIdentifier to use backticks (identical to the previous output) and provides ClickHouse-specific escapeStringLiteral (more robust than the manual ''). So identifiers are behavior-preserving and the user-value escaping is strictly safer.

Verification

  • xcodebuild -scheme TablePro build succeeds.
  • swiftlint --strict clean.

Notes

  • No test: the change delegates to the driver's quoteIdentifier/escapeStringLiteral (the tested units); the view's inline SQL lives in private SwiftUI methods.
  • No CHANGELOG: output is equivalent/more-correct ClickHouse SQL, no observable user change.
  • ABI bump: no. Docs: no.

@datlechin datlechin merged commit 043b279 into main May 30, 2026
3 of 4 checks passed
@datlechin datlechin deleted the fix/clickhouse-partition-sql-escaping branch May 30, 2026 06:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant