-
Notifications
You must be signed in to change notification settings - Fork 6
Add tuple support #87
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds ClickHouse Tuple handling and adjusts Array scanning to avoid panics when scanning arrays whose concrete Go type is not []any (e.g., []map[string]any), resolving reflect.Set assignability errors.
Changes:
- Add a
Tupletype handler and recognizeTuple(...)inunwrapCHType. - Change
Arrayscan targets/readers to useanyinstead of[]anyto accept concrete slice types returned by the driver. - Register the new
Tuplehandler in the ClickHouse type handler map.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func handleTuple(nullable bool) (any, func() any) { | ||
| if nullable { | ||
| var p *any | ||
| return &p, func() any { | ||
| if p == nil || *p == nil { |
Copilot
AI
Feb 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New code in handleTuple uses spaces for indentation and doesn’t match the rest of this Go file’s gofmt/tab formatting. Please run gofmt (or reformat this block) so indentation and brace alignment are consistent.
| func handleArray(nullable bool) (any, func() any) { | ||
| if nullable { | ||
| var p *[]any | ||
| return &p, func() any { | ||
| if p == nil { | ||
| return nil | ||
| } | ||
| return *p | ||
| } | ||
| } | ||
| var v []any | ||
| return &v, func() any { return v } | ||
| if nullable { | ||
| var p *any | ||
| return &p, func() any { | ||
| if p == nil || *p == nil { | ||
| return nil | ||
| } | ||
| return *p | ||
| } | ||
| } | ||
| var v any | ||
| return &v, func() any { return v } |
Copilot
AI
Feb 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
handleTuple and handleArray now contain identical scan-target logic. Consider factoring this into a shared helper (e.g., reuse makeScanTargetany or introduce a dedicated handler for complex/unknown types) to avoid duplication and keep future fixes in one place.
| if strings.HasPrefix(s, "Tuple(") { | ||
| return "Tuple", nullable | ||
| } |
Copilot
AI
Feb 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The added Tuple() branch uses space-indentation and is misaligned vs the surrounding gofmt style. Please gofmt/reformat this block for consistency.
Clickhouse doesn't support Tuple or Array(Tuple...)) data types. It caused panic
reflect.Set: value of type []map[string]interface {} is not assignable to type []interface{}The major reason for that was that handle array used var v []any as a result v couldn't be []map[string]interface {}. To fix that array right now expect any value because interface{} can be anything. It can be []map[string]interface {} and can be []string{} etc.
This PR fixes panic. General panic handler for all requests will be added in next PR.
Testing was done on local machine with multiple Clickhouse queries