fix(ibis): fix ClickHouse Decimal/Numeric and Nullable Type Mapping#1423
fix(ibis): fix ClickHouse Decimal/Numeric and Nullable Type Mapping#1423goldmedal merged 1 commit intoCanner:mainfrom
Conversation
📝 WalkthroughWalkthroughAdded support for numeric column types in ClickHouse type mapping. Extended the type transformation logic to recognize decimal and numeric prefixes, handle Nullable type wrappers, and map them to corresponding RustWrenEngineColumnType values. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
goldmedal
left a comment
There was a problem hiding this comment.
Thanks @MarceloNunesAlves. Makes sense to me 👍
This PR addresses an issue in the ClickHouse data source connector where
Decimal,Numeric, andNullabledata types were not being correctly mapped to Wren Engine's internal types.Previously, the type mapping relied on exact string matches. When ClickHouse returned parameterized types like
Decimal(15,2), the lookup inCLICKHOUSE_TYPE_MAPPINGwould fail, resulting in anUNKNOWNtype and a warning log.Changes
Decimal(18,4)todecimal) before performing the dictionary lookup.Numeric**: Addednumericas a valid alias fordecimalin the ClickHouse type mapping.Nullable(T)wrapper by extracting the inner typeTbefore mapping.CLICKHOUSE_TYPE_MAPPINGto ensure all variations of decimal and numeric types are captured.Problem Flow (Example)
Nullable(Decimal(15,2))Nullable→Decimal(15,2)decimaldecimalinCLICKHOUSE_TYPE_MAPPING.DECIMALtype. ✅Checklist
Summary by CodeRabbit
Improvements