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.

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

Fix Nested Map Column Type Parsing Bug #336

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ehddnr301
Copy link

@ehddnr301 ehddnr301 commented Sep 8, 2024

@xzkostyan

Hello! It's my first pull request.
I want to fix bug for Nested Map columns.

I'm currently using a datahub, and I think modifying the code will make the column types visible

Please feel free to review the changes and provide any feedback or suggestions for improvement.

related datahub issue is datahub-project/datahub#9079

  • Additional

Test Table DDL

CREATE TABLE default.deeply_nested_map_table
(
    `id` Int32,
    `deep_map` Map(String, Map(String, Map(String, Map(String, Map(String, Nullable(Int32)))))),
    `other_data` Map(String, Map(String, Map(String, Map(String, Nullable(String)))))
)
ENGINE = MergeTree
ORDER BY id

Before Edit
image

  • TypeError: Map.__init__() missing 1 required positional argument: 'value_type'

After Edit
image

Checklist:

  • Add tests that demonstrate the correct behavior of the change. Tests should fail without the change.
  • Add or update relevant docs, in the docs folder and in code.
  • Ensure PR doesn't contain untouched code reformatting: spaces, etc.
  • Run flake8 and fix issues.
  • Run pytest no tests failed. See https://clickhouse-sqlalchemy.readthedocs.io/en/latest/development.html.

- Resolved xzkostyan#269 by ensuring the parser now tracks bracket levels and splits types accordingly.
@ehddnr301
Copy link
Author

@xzkostyan

Hello, I just wanted to kindly follow up on my pull request #336.

Please let me know if there's anything I can clarify or improve. Thanks for your time!

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.

Reflection fails on complex nested types
1 participant