fix(datasets): share the cache of Ibis connections

[deepyaman]

Description

Close #935

Development notes

The ConnectionMixin can also be used by something like pandas.SQLQueryDataset and pandas.SQLTableDataset to share a single connection cache. While this has some value (prevent storing the connections twice, if the user is using both datasets), it's less functionally important as in this specific Ibis case.

Checklist

• Opened this PR as a 'Draft Pull Request' if it is work-in-progress
• Updated the documentation to reflect the code changes
• Added a description of this change in the relevant RELEASE.md file
• Added tests to cover my changes
• Received approvals from at least half of the TSC (required for adding a new, non-experimental dataset)

[noklam]

I haven't looked into this at all, but my intuition is that it stems from #842 (comment); because the connections are different, temporary tables are not shared.

I can try to look into creating a shared cache, but no other dataset follows this pattern (they are pretty independent the way they're currently designed); not sure if @merelcht @astrojuanlu @idanov @noklam any of you may have thoughts on this. #935 (comment)

Quoting this for future reference. This is not just a refactoring, but necessary as temp table are only accessible by the same connection.

[astrojuanlu]

Non blocking comment from the peanut gallery: Are there other ways of doing this without using Mixins?

I think we should do less inheritance, not more. Let alone multiple inheritance.

[deepyaman]

Non blocking comment from the peanut gallery: Are there other ways of doing this without using Mixins?

I think we should do less inheritance, not more. Let alone multiple inheritance.

I'm not 100% sure what the best alternative would be, and would need to do some reading/experimentation to figure it out. That said, I think this can definitely be refactored down the road, as it doesn't update the public interface.

Edit: https://python-patterns.guide/gang-of-four/composition-over-inheritance/ also has some ideas? But I haven't looked into it sufficiently to see what I may like.

[deepyaman]

Blocked by #949

[merelcht]

Happy with solution 👍 I do agree it would be good to see if we can achieve the same without inheritance, but since it's now only used between the Ibis datasets I don't see it as too big of an issue.

Should this be added to the release notes?

[deepyaman]

Should this be added to the release notes?

Yes, will do!