Giorgi / DuckDB.NET

Bindings and ADO.NET Provider for DuckDB
https://duckdb.net
MIT License
343 stars 62 forks source link

Support shared in-memory databases #88

Closed yxmm-wxe closed 1 year ago

yxmm-wxe commented 1 year ago

Try to implement #86.

Giorgi commented 1 year ago

Thanks, I'll probably have time towards the end of the week to review it.

yxmm-wxe commented 1 year ago

@Giorgi It was found that Build library (windows-latest) and Build library (macos-12) are successful, while Build library (ubuntu-20.04) failed.

Comparison found that pack and Upload Artifacts are disabled in Build library (windows-latest) and Build library (macos-12), while they are enabled in Build library (ubuntu-20.04).

Is there anything I can do about this?

Giorgi commented 1 year ago

I added a condition to the CI and it should skip that step now for pull request builds. Can you pull the changes from my develop branch to your fork?

coveralls commented 1 year ago

Pull Request Test Coverage Report for Build 4591753439


Changes Missing Coverage Covered Lines Changed/Added Lines %
DuckDB.NET.Data/Internal/ConnectionManager.cs 10 11 90.91%
<!-- Total: 37 38 97.37% -->
Totals Coverage Status
Change from base Build 4590505173: 0.4%
Covered Lines: 705
Relevant Lines: 812

💛 - Coveralls
Giorgi commented 1 year ago

@yxmm-wxe

It's creating on-disk databases DataSource=:memory:?cache=shared connection string.

image

yxmm-wxe commented 1 year ago

@Giorgi Sorry for my carelessness, something is missing. I have fixed the issue.

Giorgi commented 1 year ago

Thanks @yxmm-wxe it works fine now. I think the Duplicate method isn't required. You can create a new connection with the same connection string if you need to. What do you think?

yxmm-wxe commented 1 year ago

@Giorgi Yes, what you said could work for DataSource=:memory:?cache=shared and other on-disk databases.

But according to the current design, it will create an isolated in-memory database via DataSource=:memory:, and only one connection can be used. I don't think this can be used as a special case.

The Duplicate method can only create another connection for the existing isolated in-memory database. And it was also supported in JDBC and Python.

Giorgi commented 1 year ago

I see but why not use the connection that you have? Why the need to Duplicate it?

yxmm-wxe commented 1 year ago

@Giorgi I found [duckdb_connect instruction](https://duckdb.org/docs/archive/0.7.1/api/c/connect#:~:text=With%20the%20duckdb_database%20handle%2C%20you%20can%20create%20one%20or%20many%20duckdb_connection%20using%20duckdb_connect().%20While%20individual%20connections%20are%20thread%2Dsafe%2C%20they%20will%20be%20locked%20during%20querying.%20It%20is%20therefore%20recommended%20that%20each%20thread%20uses%20its%20own%20connection%20to%20allow%20for%20the%20best%20parallel%20performance.) in the official document. It is recommended that each thread uses its own connection to allow for the best parallel performance.

colombod commented 1 year ago

is this feature released on nuget.org?

Giorgi commented 1 year ago

Not yet but you can get it from GitHub Packages

colombod commented 1 year ago

Not yet but you can get it from GitHub Packages

I would need it from nuget.org, I am building a kernel around your package to make it usable in Polyglot Notebooks (https://github.com/colombod/dotnet-interactive-extension-lab/pull/31) so far all look, would love to talk with you if you have time

Giorgi commented 1 year ago

We can chat at DuckDB Discord server.