Skip to content
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 populating of TYPE_SCHEMA column in StructuredTypeMembers metadata #1500

Merged
merged 4 commits into from May 28, 2022

Conversation

tf-micwil
Copy link
Contributor

Fixes for #1498

  • Updated the FROM clause on the SELECT statement for StructuredTypeMembers metadata to use the schema name from sys.table_types.
  • Added StructuredTypeMembers to SqlClientMetaDataCollectionNames
  • Added unit tests for StructuredTypeMembers

I was unable to run the tests as I don't have access to a required nuget package in test project.

@dnfadmin
Copy link

dnfadmin commented Feb 1, 2022

CLA assistant check
All CLA requirements met.

@Wraith2
Copy link
Contributor

Wraith2 commented Feb 2, 2022

Looks good in general.

The nuget comes from the package source in the nuget.config file in the src directory, everything below that directory should pick it up. I can run tests locally but I've been doing it for a while so if something has broken I might already have the nuget packages locally. Specifically which packages failed to restore?

@tf-micwil
Copy link
Contributor Author

@Wraith2: Microsoft.Data.SqlClient.ManualTesting.Test is reporting that Microsoft.DotNet.RemoteExecutor (5.0.0-beta.20206.4) is not available in the dotnet-public package source.

@Wraith2
Copy link
Contributor

Wraith2 commented Feb 2, 2022

I don't have that package locally and I haven't had that problem (but then i haven't opened any PR's in a while since I.m waiting.). Remote executor is needed for tests that are going to run in the ci as an external process so you may not need it to check your work locally.

@JRahnama
Copy link
Member

/azp run

@azure-pipelines
Copy link

You have several pipelines (over 10) configured to build pull requests in this repository. Specify which pipelines you would like to run by using /azp run [pipelines] command. You can specify multiple pipelines using a comma separated list.

@codecov
Copy link

codecov bot commented May 27, 2022

Codecov Report

Merging #1500 (582626b) into main (845902c) will increase coverage by 5.93%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1500      +/-   ##
==========================================
+ Coverage   65.47%   71.41%   +5.93%     
==========================================
  Files         295      295              
  Lines       62125    62126       +1     
==========================================
+ Hits        40678    44368    +3690     
+ Misses      21447    17758    -3689     
Flag Coverage Δ
addons 92.38% <ø> (+92.38%) ⬆️
netcore 74.89% <100.00%> (+5.88%) ⬆️
netfx 69.22% <100.00%> (+5.59%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...Data/SqlClient/SqlClientMetaDataCollectionNames.cs 100.00% <100.00%> (ø)
...nt/src/Microsoft/Data/ProviderBase/TimeoutTimer.cs 71.42% <0.00%> (-3.58%) ⬇️
.../Microsoft/Data/SqlClient/SNI/SNIMarsConnection.cs 76.72% <0.00%> (-0.63%) ⬇️
...c/Microsoft/Data/SqlClient/TdsParserStateObject.cs 81.85% <0.00%> (-0.08%) ⬇️
...re/src/Microsoft/Data/SqlClient/SNI/SNINpHandle.cs 73.29% <0.00%> (ø)
...rc/Microsoft/Data/ProviderBase/DbConnectionPool.cs 86.36% <0.00%> (ø)
...crosoft/Data/SqlClient/SqlInternalConnectionTds.cs 73.54% <0.00%> (+0.09%) ⬆️
...crosoft/Data/SqlClient/SqlInternalConnectionTds.cs 68.26% <0.00%> (+0.17%) ⬆️
...c/Microsoft/Data/SqlClient/SqlClientEventSource.cs 80.13% <0.00%> (+0.34%) ⬆️
...SqlClient/src/Microsoft/Data/SqlClient/SqlEnums.cs 77.37% <0.00%> (+0.63%) ⬆️
... and 55 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 845902c...582626b. Read the comment docs.

@DavoudEshtehari DavoudEshtehari added the 🆕 Public API Use this label for new API additions to the driver. label May 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🆕 Public API Use this label for new API additions to the driver.
Projects
SqlClient issues
Awaiting triage
Development

Successfully merging this pull request may close these issues.

None yet

6 participants