-
Notifications
You must be signed in to change notification settings - Fork 14
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: resolve compatibilities for Metabase (#340) #352
Conversation
1c3f1b2
to
5b3681e
Compare
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.
Copilot reviewed 5 out of 10 changed files in this pull request and generated no comments.
Files not reviewed (5)
- pgserver/connection_data.go: Evaluated as low risk
- pgserver/duck_handler.go: Evaluated as low risk
- pgserver/iter.go: Evaluated as low risk
- catalog/provider.go: Evaluated as low risk
- pgserver/connection_handler.go: Evaluated as low risk
Comments suppressed due to low confidence (3)
pgserver/in_place_handler.go:124
- Ensure that the new InPlaceHandler struct and its methods are covered by tests.
type InPlaceHandler struct {
pgserver/in_place_handler.go:140
- Ensure that the selectionConversions logic is covered by tests, especially the regex patterns and query conversions.
var selectionConversions = []SelectionConversion{
pgserver/in_place_handler.go:374
- Ensure that the updated shouldQueryBeHandledInPlace function is covered by tests, including all its call sites.
func shouldQueryBeHandledInPlace(h *ConnectionHandler, sql *ConvertedStatement) (bool, error) {
…session level pgtypes.Map for each session to encode results
5519485
to
b84fc11
Compare
The failed |
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.
Thanks! Look good. It's the first BI tool that we can seamlessly integrate with. Congratulations!
This PR addresses the issues of the compatiblities between MyDuck and Metabase.
The other BI tools are not tested yet. If there still be compatibility issues for them, we should open another PR for them.
Ref #340
Resolve #342