Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Reflection fails on complex nested types #269

Open
hsheth2 opened this issue Oct 27, 2023 · 0 comments · May be fixed by #336
Open

Reflection fails on complex nested types #269

hsheth2 opened this issue Oct 27, 2023 · 0 comments · May be fixed by #336

Comments

@hsheth2
Copy link

hsheth2 commented Oct 27, 2023

Describe the bug

For a table DDL like this:

CREATE TABLE IF NOT EXISTS bugtable on cluster 'local' (
	id Int,
	metadata Map(String, Map(String, Nullable(String)))
) ENGINE = MergeTree()
order by id

Reflection using get_columns() fails with Map.__init__() missing 1 required positional argument: 'value_type'

It looks like the issue is in this code

clickhouse-sqlalchemy/clickhouse_sqlalchemy/drivers/base.py

Lines 242 to 248 in a3140a9

elif spec.startswith('Map'):
inner = spec[4:-1]
coltype = self.ischema_names['_map']
inner_types = [
self._get_column_type(name, t.strip())
for t in inner.split(',')
]
- the .split(',') is getting an input of String, Map(String, Nullable(String)) and splits it to [String, 'Map(String', 'Nullable(String))'] - it doesn't respect the parentheses. It could probably be replaced with a .split(',', maxsplit=1) to fix this issue (unless the key type was also complex, but that seems less likely)

Expected behavior
This should get reflected properly.

Originally discovered in the DataHub slack community: https://datahubspace.slack.com/archives/CUMUWQU66/p1698241313514559?thread_ts=1698112545.709559&cid=CUMUWQU66
@hsheth2 hsheth2 mentioned this issue Nov 1, 2023
Open
ehddnr301 added a commit to ehddnr301/clickhouse-sqlalchemy that referenced this issue Sep 8, 2024
@ehddnr301
fix: parsing issue for nested Map types
3681d8a 
- Resolved xzkostyan#269 by ensuring the parser now tracks bracket levels and splits types accordingly.
ehddnr301 added a commit to ehddnr301/clickhouse-sqlalchemy that referenced this issue Sep 8, 2024
@ehddnr301
fix: parsing issue for nested Map types
b4f62fe 
- Resolved xzkostyan#269 by ensuring the parser now tracks bracket levels and splits types accordingly.
@ehddnr301 ehddnr301 linked a pull request Sep 8, 2024 that will close this issue
Open
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix Nested Map Column Type Parsing Bug ehddnr301/clickhouse-sqlalchemy
1 participant
@hsheth2