Implementation of the Connection.GetSchema of the DataSourceInformation collection#190
Implementation of the Connection.GetSchema of the DataSourceInformation collection#190magneto-81 wants to merge 5 commits intoGiorgi:developfrom
Conversation
Giorgi
left a comment
Giorgi
left a comment
There was a problem hiding this comment.
Thanks, added some minor comments.
DuckDB.NET.Data/DuckDBSchema.cs
Outdated
| collectionName.ToUpperInvariant() switch | ||
| { | ||
| "METADATACOLLECTIONS" => GetMetaDataCollections(), | ||
| "DATASOURCEINFORMATION" => GetDataSourceInformation(NativeMethods.Startup.DuckDBLibraryVersion().ToManagedString(false)), |
There was a problem hiding this comment.
Replace with connection.ServerVersion
DuckDB.NET.Data/DuckDBSchema.cs
Outdated
| serverVersion, | ||
| serverVersion, | ||
| GroupByBehavior.Unrelated, | ||
| "(^\\[\\p{Lo}\\p{Lu}\\p{Ll}_@#][\\p{Lo}\\p{Lu}\\p{Ll}\\p{Nd}@$#_]*$)|(^\\[[^\\]\\0]|\\]\\]+\\]$)|(^\\\"[^\\\"\\0]|\\\"\\\"+\\\"$)", |
There was a problem hiding this comment.
Can you add comments explaining what these regexes do/mean and where they come from?
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## develop #190 +/- ##
===========================================
+ Coverage 88.38% 88.73% +0.35%
===========================================
Files 54 54
Lines 1756 1802 +46
Branches 239 239
===========================================
+ Hits 1552 1599 +47
+ Misses 146 145 -1
Partials 58 58 ☔ View full report in Codecov by Sentry. |
Pull Request Test Coverage Report for Build 9157409247Details
💛 - Coveralls |
|
Is it possible to test it some other way? I.E what uses |
|
Connection.GetSchema("DataSourceInformation") is used by generic software which, through the ado.net providers, allows its use. The information returned by this primitive is used to generate statements in a generic way (see for example DbMetaDataColumnNames.ParameterMarkerPattern). |
|
Can you add a test that uses those APIs (that call |
| // https://duckdb.org/docs/sql/query_syntax/prepared_statements | ||
| { | ||
| "\\.", | ||
| "DuckDB", |
There was a problem hiding this comment.
Here you return “DuckDB”, but test checks for “duckdb” (lowercase)
|
5 participants
No description provided.