Skip to content

fix(clickhouse): encode schema names in JDBC URLs - #2692

Merged
openai0229 merged 4 commits into
OtterMind:mainfrom
HandSonic:fix/clickhouse-schema-url-encoding
Sep 9, 2026
Merged

openai0229 merged 4 commits into
OtterMind:mainfrom
HandSonic:fix/clickhouse-schema-url-encoding

Conversation

@HandSonic

@HandSonic HandSonic commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Encode the selected ClickHouse schema as one UTF-8 JDBC URL path segment before reconnecting.
  • Prevent /, ?, #, spaces, Unicode, plus signs, and literal percent text from changing URL structure or the selected schema.
  • Preserve the existing JDBC query suffix and add focused regression coverage.

Validation

  • Focused regression tests: 3 passed.
  • Full ClickHouse module tests: 46 passed, 0 failed.
  • ClickHouse reactor package: passed.
  • Fork code CI: frontend, backend, repository checks, Java/JavaScript CodeQL, SBOM, and license checks passed.
  • Independent adversarial review: no findings.

Follow-up regression fix related to #2685 for schema names containing reserved URL characters. This intentionally does not close that issue; the separate IPv6 URL fix is in #2690.

Latest-main verification (2026-09-04)

  • Rebased onto 144a04ee2; full ClickHouse plugin suite passed (55 tests), including IPv6, user-info, query/fragment, schema encoding, and idempotence cases.n- git diff --check` and pairwise integration analysis passed.

@HandSonic
HandSonic requested a review from openai0229 as a code owner August 12, 2026 02:26
@openai0229 openai0229 moved this to In Review in Chat2DB Community Aug 12, 2026
@HandSonic
HandSonic force-pushed the fix/clickhouse-schema-url-encoding branch from 05a46c2 to d4e8177 Compare August 12, 2026 16:45
@HandSonic
HandSonic force-pushed the fix/clickhouse-schema-url-encoding branch from d4e8177 to 7da9798 Compare August 19, 2026 20:05

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

Blocking integration issue: this branch still applies encoding on top of the old implementation. That implementation is exactly what #2690 replaces because it cannot reliably preserve IPv6 authorities, no-path URLs, fragments, and other URL shapes. Merging the two fixes independently leaves overlapping implementations and an avoidable conflict.\n\nPlease base this change on #2690's helper and encode the schema as one UTF-8 path segment inside that helper. Keep the reserved-character tests from this PR and the IPv6/query/fragment tests from #2690 so there is one URL-rewrite implementation.

@openai0229
openai0229 dismissed their stale review August 20, 2026 03:45

Superseded by a corrected review because shell quoting corrupted the submitted text.

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

Blocking integration issue: this branch still applies encoding on top of the old url.split(connectAddress) implementation. That implementation is exactly what #2690 replaces because it cannot reliably preserve IPv6 authorities, no-path URLs, fragments, and other URL shapes. Merging the two fixes independently leaves overlapping implementations and an avoidable conflict.

Please base this change on #2690's replaceDatabaseInJdbcUrl helper and encode the schema as one UTF-8 path segment inside that helper. Keep the reserved-character tests from this PR and the IPv6/query/fragment tests from #2690 so there is one URL-rewrite implementation.

@HandSonic
HandSonic force-pushed the fix/clickhouse-schema-url-encoding branch from 7da9798 to 0d2b7b3 Compare August 22, 2026 15:05
@HandSonic
HandSonic requested a review from Aias00 as a code owner August 22, 2026 15:05
@HandSonic
HandSonic force-pushed the fix/clickhouse-schema-url-encoding branch from 4e61dee to 2ebd148 Compare September 3, 2026 22:55
@HandSonic

Copy link
Copy Markdown
Contributor Author

Addressed on the current head: there is now one
eplaceDatabaseInJdbcUrl implementation with both the schema-as-one-UTF-8-segment cases and the IPv6/no-path/query/fragment/idempotence cases. The strict-subset #2690 was closed. The full ClickHouse suite passes (55 tests) on current main. Please re-review when convenient.

@openai0229
openai0229 merged commit 5a17665 into OtterMind:main Sep 9, 2026
16 of 17 checks passed
@openai0229 openai0229 moved this from In Review to Done in Chat2DB Community Sep 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants