Skip to content

fix: parse Map key types containing commas - #1941

Open
fallintoplace wants to merge 1 commit into
ClickHouse:mainfrom
fallintoplace:fix/map-nested-key-type-parsing
Open

fix: parse Map key types containing commas#1941
fallintoplace wants to merge 1 commit into
ClickHouse:mainfrom
fallintoplace:fix/map-nested-key-type-parsing

Conversation

@fallintoplace

@fallintoplace fallintoplace commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Allow Map key types to contain commas in nested parameters. For example, the driver now parses this valid type correctly:

Map(DateTime64(3, 'UTC'), String)

Problem

Map.parse split key and value types at the first comma. For the example above, it treated DateTime64(3 as the key type and failed before any query could run. The existing Enum-specific workaround covered only one form of nested comma.

Fix

Find the first top-level comma while ignoring nested parentheses and quoted content. This handles parameterized key types generally, preserves Enum labels containing parentheses, and removes the Enum special case. The parsing flow was also flattened so invalid and non-comparable key types return early.

Tests

Unit coverage includes:

  • DateTime64 keys with timezone parameters
  • Enum keys, including a label containing )
  • simple Map types
  • nested value types
  • malformed Map definitions
go test ./...

Checklist

  • Unit tests covering the regression were added

@fallintoplace fallintoplace changed the title Fix Map parsing for nested key types fix: parse Map types with nested key parameters Jul 30, 2026
@fallintoplace
fallintoplace force-pushed the fix/map-nested-key-type-parsing branch from 2685027 to 43e4cf2 Compare July 30, 2026 17:40
@fallintoplace
fallintoplace force-pushed the fix/map-nested-key-type-parsing branch from 43e4cf2 to dcf8a4c Compare July 30, 2026 17:59
@fallintoplace fallintoplace changed the title fix: parse Map types with nested key parameters fix: parse Map key types containing commas Jul 30, 2026
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